Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1whUIx-000Fvd-2f for pgsql-hackers@arkaria.postgresql.org; Wed, 08 Jul 2026 15:35:20 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1whUIw-007pQD-18 for pgsql-hackers@arkaria.postgresql.org; Wed, 08 Jul 2026 15:35:19 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1whUIv-007pQ4-35 for pgsql-hackers@lists.postgresql.org; Wed, 08 Jul 2026 15:35:18 +0000 Received: from mail-pj1-x1034.google.com ([2607:f8b0:4864:20::1034]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.98.2) (envelope-from ) id 1whUIt-00000000IGe-2rif for pgsql-hackers@postgresql.org; Wed, 08 Jul 2026 15:35:18 +0000 Received: by mail-pj1-x1034.google.com with SMTP id 98e67ed59e1d1-37f9e2642deso96636a91.1 for ; Wed, 08 Jul 2026 08:35:15 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1783524914; cv=none; d=google.com; s=arc-20260327; b=LlznVu4gOpJ8aqXnMSUPZXA70pzxzro1+S9qbCK3d2f62GmxOTff++t+NaCOWdqwQk U1I8pTEIMqNvTWSwzv2G5X3fgclhQFNrTmYkavSZE1t+BEfqvVI5P8nTLIVXCi9Fpzyj s37QmHpZckMUFjygecRE8a51bC78qRWbi7OOAfzhrBHWdbmKt7seibc1XbpEnhKqU2mm liI6uJnSSuxjmrXpqUWZrRDTrvGoSHve+s1KVN+QUgXPwYsVJhxg/1T5C+ncp2jyp6Vb M/HfXWA4qE0FClH5zvzTFK+b9uWy696bSlPeSibl5wnkOdNOPkTWcCLI3L2m5UxJsK74 QVsg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20260327; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:dkim-signature; bh=58bAV9E+Fe7waUI1iqeq+2KUGIU+XFCMmgatNG330Ug=; fh=nwhwGo26it7ENXWGh6JOw0Radp1GiolGZhPuvGISfcA=; b=PTCk5w5Ox91lNUhZdVL+fHLPFHFGKa7MbMN9juVN1EZTXiYJq3rnE+9Ko7wdCE0XQh 1pN3vmtCJVL0Lr2OnEpglmId6BAYHl/ApoLCfFsb/RrhJMING8IzByQacubkfBBUS4Ii 0k953HpSEy+Cpzuxns5xtuJPuMw7ZoUb4pfXLFtHLnTxnYvdhD8mM3TNaDnTg6QV6jB/ hJ1yj3tnuCaX3By0tTb57Nv5oXPaQrpEcBl5hY9SO4ADGuXoV2/qC93GsKUjOHjd8IBZ yR3ry5WtFHjYDCRAgxy7h8phwYxTYnP6+Z1IiqyLJqcKswDlgmogZE7R8EpHIyyguTOb CgUQ==; darn=postgresql.org ARC-Authentication-Results: i=1; mx.google.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1783524914; x=1784129714; darn=postgresql.org; h=content-type:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:from:to:cc:subject:date:message-id:reply-to :content-type; bh=58bAV9E+Fe7waUI1iqeq+2KUGIU+XFCMmgatNG330Ug=; b=B54/9KEkJG5Cvo7+RhPef1WrDkt0U3Q9L8fJ+W4OF28LFa21eksfYuG1qAOG3GlSbR Pa0m0BppbjcdCvlkqN+tC5BqZL7kitPRTiqlTfly1PCiNy5FbS2Qi04wNcZYmntNSnKT pWhes8Fh3TVMIR9J8I9vEB5jG9wkrka3rpg7/I1dFCRh0rmkn6yuofVYAUA6zx12cHj3 UODFZecYqkkjKS6fo1CoDekSk+XxsVi5Z1rpAnErnhh95CL/GsTL8TtuJft8hrzAxAdy AxhVbvnyc2PjDrkxcFTcrZwYTsbo01hLkW99x8xpIrsK8xhM1KtSiWZSeWEokKladusX KxhQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1783524914; x=1784129714; h=content-type:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to:content-type; bh=58bAV9E+Fe7waUI1iqeq+2KUGIU+XFCMmgatNG330Ug=; b=GaBGGxOEgqyLIf9JeGbbYAYhWFic0TEoyoKVs2S5IQyANeE3wxlKB/jIvpAScnM92r N98RxoLloVCAcyy2bzVd8ZRuJUdNpnTOJiWExec6ETN8Fe49I8L1NdmlJIYJ2nS6gVt+ 0rNQsaPlhlM9XCBR+WZjQySgzKpYnJ8wfYgh0YQjpW32uPsp3EsgM93/TRaNbM3/WeSA rFFwri6XCliax8W2W2D8akqa/Rggc5drHGDSc69yG334fpMaPPBdlNq8FxhwTyyfj+7x Lsv9kdmKAtRN1AzVa6vrdzJZ/z7e0erZqqLLhPZx2o5bfdxy0tWyU67ej8S5RmgOZ4WS 3gDg== X-Forwarded-Encrypted: i=1; AHgh+Ro0tPowp7mxYJ2nVH0EByQSyCejRQ0VjN6WSqjB9hAy78PDtvCEFblnIMTFIEIGWAmmJvsfoCFSzLPPHP0z@postgresql.org X-Gm-Message-State: AOJu0YzEAoy25dlr6yOK9mYe1r4IDy+Hj1qtxjA9zhhaldHVIZ/DIQPh +DduuBiviwTIpBcqHnvs6e0oXUgopgzijKkXEr9o2yQPyXD62e5eB1Ezm/B626z/W8Nib9t2pjs 1JT2WUZFBznsP733OgIy9bN6gfxnawgU= X-Gm-Gg: AfdE7clAw5JgHKAwXPe7bd0Jj21DC9a428IhCKne/5k3wkY3/EGlmnB5424sC+sCime LzhNSr4eX7nHClptkOYQl1OtMDrFi5wJX4pvlrP/P36KqGdWWzGrmqHodIO580+QJ8DAfNUbFcw YohlTrALQUO9g+nz89xx8sisIJcBazuBNWQs4ckWldSR5XXS8laIOxe5vxpaFSE2uB3KOLetyqL 6Sz1XwRD2IFtJvS+4m7BrVMuSzf1I7lD8soBUhnYUSSTIBIwAH63itJOUaurQSy9QKCO1Oh X-Received: by 2002:a17:90b:4fd0:b0:381:2d9a:447b with SMTP id 98e67ed59e1d1-38a20e41816mr73597a91.4.1783524913550; Wed, 08 Jul 2026 08:35:13 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Nikita Malakhov Date: Wed, 8 Jul 2026 18:35:00 +0300 X-Gm-Features: AVVi8CeboHYJ8LSGgPw9VpWzFWu9MSidzKlZu6N0-pFWKXUJJhS6HfrQwuBbJoM Message-ID: Subject: Re: SQL/JSON json_table plan clause To: Alexander Korotkov Cc: Amit Langote , pgsql-hackers Content-Type: multipart/alternative; boundary="000000000000eb970706561b4025" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --000000000000eb970706561b4025 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi! Alexander, thank you for your participation. I am very sorry, I've seen your previous email but was busy with other tasks and didn't have time to work on your notes. Thank you very much! On Wed, Jul 8, 2026 at 2:39=E2=80=AFAM Alexander Korotkov wrote: > Hi! > > On Sun, Jul 5, 2026 at 12:02=E2=80=AFAM Alexander Korotkov > wrote: > > On Fri, May 15, 2026 at 2:53=E2=80=AFPM Nikita Malakhov > wrote: > > > Again, Alexander and Amit, thanks for the review. I've rebased the > patch and made > > > some changes according to Alexander's notes. I've slightly rearranged > files between > > > patches for it to be easier to review, so now there are 3 patches: > > > v23-0001-json-table-plan-clause.patch - main code changes > > > v23-0002-json-table-plan-tests.patch - test cases and out file change= s > > > v23-0003-json-table-plan-docs.patch - docs package > > > > > > <...> > > > >1. IsA(planstate, JsonTableSiblingJoin) is wrong. planstate is not = a > > > >node, thus IsA() can't be applied to it. You should instead do > > > >IsA(planstate->plan, JsonTableSiblingJoin). It wasn't catched, > > > >because regression tests don't exercise this branch. So, you also > > > >need to improve the coverage. > > > - done; > > > > > > >2. get_json_table() with patch uses JSON_BEHAVIOR_EMPTY as the defau= lt > > > >value for deparsing, while parsing still uses > > > >JSON_BEHAVIOR_EMPTY_ARRAY. Looks plain wrong. I'm not sure what is > > > >intention here. > > > - looks like leftover from older version, changed to default behavior= , > so unnecessary > > > emission of ERROR is omitted; > > > > > > >3. PLAN clause is always emitted during deparsing even if user didn'= t > > > >specify anything. I would prefer to skip PLAN clause in this case > > > >unless there is strong reason to do the opposite (this reason must b= e > > > >pointed if any). > > > - done, this made test cases much more readable; > > > > > > >4. Unused typedefs in src/tools/pgindent/typedefs.list: > > > >JsonTableScanState, JsonPathSpec, JsonTablePlanStateType, > > > >JsonTableJoinState. > > > - done; > > > > > > >5. Empty comment in JsonTablePlanState definition. Pointed by Amit, > > > >but not fixed. > > > - done; > > > > > > >6. Rename passingArgs to passing_Args for no reason in > parse_jsontable.c. > > > - done; > > > > > > >7. Patch lacks documentation (also pointed by Amit) > > > - done, documentation is provided in separate patch; > > > > > > >8. Patch could use pgindent run. > > > - not done yet but would provide a newer version with it. > > > > Thank you. I've made following additional changes. > > > > 1. Actually, I don't see my previous item 3 fully addressed. We > > still had PLAN clause deparsed in the case user didn't specify it. I > > see this as an undesirable behavior. And there are at least two ways > > to fix this: don't output PLAN clause if it's not the default, > > remember if user specified as a special flag or even original clause. > > I found first way (don't output default) more appealing as it don't > > require additional struct members and we already do this in other > > similar cases (NULLS FIRST/LAST, ON EMPTY/ON ERROR). So, I did this > > for PLAN clause. > > 2. Simplification for JsonTablePlanNextRow(): no reason to check > > (planstate->advanceNested || planstate->nested) if we already issued > > break for (planstate->nested =3D=3D NULL). > > 3. Similar simplification for transformJsonTableNestedColumns(). > > I made some additional changes to the patch. In the previous versions > PLAN qual required name for root path, but didn't required names for > nested paths. I've rechecked with standard, and it appears that > standard claims the opposite: name is not required for the root path, > but required for nested paths. So, I decided to allow skipping name > for both root path (as standard claims), and nested path (our > extension to standard). Corresponding changes made to code, tests and > documentation. > > Also, I merged 3 patches into 1, and edited sql_features.txt. > > I'm going to push this if no objections. > > ------ > Regards, > Alexander Korotkov > Supabase > --=20 Regards, Nikita Malakhov Postgres Professional The Russian Postgres Company https://postgrespro.ru/ --000000000000eb970706561b4025 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi!

Alexander, thank y= ou for your participation. I am very sorry, I've seen your previous
email but was busy=C2=A0with other tasks=C2=A0= and didn't have time to work on your notes.
Thank=C2=A0you very much!

On Wed, Jul 8, 2026 at 2:39=E2=80=AFAM Alexander Korotk= ov <aekorotkov@gmail.com>= wrote:
Hi!

On Sun, Jul 5, 2026 at 12:02=E2=80=AFAM Alexander Korotkov <aekorotkov@gmail.com> = wrote:
> On Fri, May 15, 2026 at 2:53=E2=80=AFPM Nikita Malakhov <hukutoc@gmail.com> wrot= e:
> > Again, Alexander and Amit, thanks for the review. I've rebase= d the patch and made
> > some changes according to Alexander's notes. I've slightl= y rearranged files between
> > patches for it to be easier to review, so now there are 3 patches= :
> > v23-0001-json-table-plan-clause.patch - main code changes
> > v23-0002-json-table-plan-tests.patch - test cases and out file ch= anges
> > v23-0003-json-table-plan-docs.patch - docs package
> >
> > <...>
> > >1. IsA(planstate, JsonTableSiblingJoin) is wrong.=C2=A0 plans= tate is not a
> > >node, thus IsA() can't be applied to it.=C2=A0 You should= instead do
> > >IsA(planstate->plan, JsonTableSiblingJoin).=C2=A0 It wasn&= #39;t catched,
> > >because regression tests don't exercise this branch.=C2= =A0 So, you also
> > >need to improve the coverage.
> > - done;
> >
> > >2. get_json_table() with patch uses JSON_BEHAVIOR_EMPTY as th= e default
> > >value for deparsing, while parsing still uses
> > >JSON_BEHAVIOR_EMPTY_ARRAY.=C2=A0 Looks plain wrong.=C2=A0 I&#= 39;m not sure what is
> > >intention here.
> > - looks like leftover from older version, changed to default beha= vior, so unnecessary
> > emission of ERROR is omitted;
> >
> > >3. PLAN clause is always emitted during deparsing even if use= r didn't
> > >specify anything.=C2=A0 I would prefer to skip PLAN clause in= this case
> > >unless there is strong reason to do the opposite (this reason= must be
> > >pointed if any).
> > - done, this made test cases much more readable;
> >
> > >4. Unused typedefs in src/tools/pgindent/typedefs.list:
> > >JsonTableScanState, JsonPathSpec, JsonTablePlanStateType,
> > >JsonTableJoinState.
> > - done;
> >
> > >5. Empty comment in JsonTablePlanState definition.=C2=A0 Poin= ted by Amit,
> > >but not fixed.
> > - done;
> >
> > >6. Rename passingArgs to passing_Args for no reason in parse_= jsontable.c.
> > - done;
> >
> > >7. Patch lacks documentation (also pointed by Amit)
> > - done, documentation is provided in separate patch;
> >
> > >8. Patch could use pgindent run.
> > - not done yet but would provide a newer version with it.
>
> Thank you.=C2=A0 I've made following additional changes.
>
>=C2=A0 1. Actually, I don't see my previous item 3 fully addressed.= =C2=A0 We
> still had PLAN clause deparsed in the case user didn't specify it.= =C2=A0 I
> see this as an undesirable behavior.=C2=A0 And there are at least two = ways
> to fix this: don't output PLAN clause if it's not the default,=
> remember if user specified as a special flag or even original clause.<= br> > I found first way (don't output default) more appealing as it don&= #39;t
> require additional struct members and we already do this in other
> similar cases (NULLS FIRST/LAST, ON EMPTY/ON ERROR).=C2=A0 So, I did t= his
> for PLAN clause.
>=C2=A0 2. Simplification for JsonTablePlanNextRow(): no reason to check=
> (planstate->advanceNested || planstate->nested) if we already is= sued
> break for (planstate->nested =3D=3D NULL).
>=C2=A0 3. Similar simplification for transformJsonTableNestedColumns().=

I made some additional changes to the patch.=C2=A0 In the previous versions=
PLAN qual required name for root path, but didn't required names for nested paths.=C2=A0 I've rechecked with standard, and it appears that standard claims the opposite: name is not required for the root path,
but required for nested paths.=C2=A0 So, I decided to allow skipping name for both root path (as standard claims), and nested path (our
extension to standard).=C2=A0 Corresponding changes made to code, tests and=
documentation.

Also, I merged 3 patches into 1, and edited sql_features.txt.

I'm going to push this if no objections.

------
Regards,
Alexander Korotkov
Supabase


--
Regards,
Nikita Malakhov
Postgr= es Professional
The Russian Postgre= s Company
<= /div> --000000000000eb970706561b4025--