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.94.2) (envelope-from ) id 1upXWU-007LVF-UU for pgsql-hackers@arkaria.postgresql.org; Fri, 22 Aug 2025 19:34:04 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1upXWU-007mSP-7E for pgsql-hackers@arkaria.postgresql.org; Fri, 22 Aug 2025 19:34:02 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1upXWT-007mSE-Po for pgsql-hackers@lists.postgresql.org; Fri, 22 Aug 2025 19:34:02 +0000 Received: from mail-ot1-x342.google.com ([2607:f8b0:4864:20::342]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.96) (envelope-from ) id 1upXWR-001Evx-2Q for pgsql-hackers@postgresql.org; Fri, 22 Aug 2025 19:34:01 +0000 Received: by mail-ot1-x342.google.com with SMTP id 46e09a7af769-74381df9401so1366600a34.0 for ; Fri, 22 Aug 2025 12:34:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1755891239; x=1756496039; darn=postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=NU6PEQaJ1p2F7I0c+sRHiy5kfFDJuSXWGwcgIMsBwl4=; b=fSHA8dSUMPDmYtXqAh9/S3ScG88oioq3pBgIDN2ay7Trz0v0KJZMhiq7um+Mb5qggJ QpbtJ9BOln46kYrWWBP+mdvNlrZMI2UaQWfcDYVMGcrfrjTrTUsblIxe0/bZJkEtvuL+ H6iQd4QKTqyufa0RMdeb+iAYTOXBiFnwxB8zRGHc74wjzR/IpoheLxtWHNifbX0T7aJX yf4FxD2HdLfIy+59erQ+68Zihz+uIJCv1iOczelJEJpRAirCp/Jos+HsS5mJl/dlLcbL Vw1Km0OZYaRJRY6EHtJWjB7Bm/l/vtpDRvRS06sSmaZdc2+JTJQB7e61Chc24P7W7Tum UbNg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1755891239; x=1756496039; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=NU6PEQaJ1p2F7I0c+sRHiy5kfFDJuSXWGwcgIMsBwl4=; b=a6COVL62f6vTAakCqKSkprP4EFRvtMHopsIDHsAerzxVQC/FbiPhVYFvIwRhpT2Oo9 UoOuMtIOZ8XFsBl6AtwCv/yIGgzO9SoGGbw75nCFPoiu2/tbfwE03er1Eu4z/0vr0UA1 VAQNvgJffOfMwqmA/90/tiS+lHFv7To1OX0xrPihjP9rqgrDNe7MJt97uv+mb//f2OUB GMWSlzai9SRX4110+TjRiyR+divl5s90B6TH65a/U98cYZu4afpn4KyLBXnygT/mBGk2 fmxiRsD8CxKQfSV5S5xSQko0DzqjTUwBosCH7bBNMLAoB1Dhvd03enn4Giag6QMO/2Qe hpQQ== X-Forwarded-Encrypted: i=1; AJvYcCWbZJVmuK/Ayh5cxICUCIVdm88yeRKzNeXn7/iB0gLeFOa/DEdQIIs7G/dJRWMLYYSL8eeuHOkcQJT4m4Qz@postgresql.org X-Gm-Message-State: AOJu0YzGHZMcqG4eYcbXv/I1ibrwmK8KqTjRb92jy4TTqh9h5VXvy16D YbXAeICde732WXdA5HZE70p3zeGVnnbTkACuKAJX3rasoO1gQN73EUAfbSZ3grzwpmKUeEhc1no PM297VBgpJVEjRs75NiJeljAHV7OTAQI= X-Gm-Gg: ASbGncvxpQNdB3AF5GQn+5TmR0TaxK2xtjbB3NmD+m7643ylgLEO8FZJMTMtHZV+0xD P9Ezb10Rid1jBkYlbpOMPpH6XSZErkYjdTfgzy20rr+qiPPfxEO3gIhdgv8VEVuCIuUcPbRn8IW 2SQ/clNkV7WaSgW+qoqUjctbLZcdEGVj/myqJpmtmK358NFAmkzysFoyrLic172c+TKgkzcJkig aj1ZijapFMuhZNAyw== X-Google-Smtp-Source: AGHT+IEj4nmFEptLSBZjI1jkYahmyk+feC1f11fxFOG4oML3ZLyD3AN11jbd8vC1cSoA+kciUkWnnOVrj/AY9jm/3m0= X-Received: by 2002:a05:6808:3307:b0:437:7577:d45f with SMTP id 5614622812f47-437852aefc4mr2284193b6e.51.1755891239533; Fri, 22 Aug 2025 12:33:59 -0700 (PDT) MIME-Version: 1.0 References: <21fa36b4-6d56-4556-b524-9e3955b922c8@eisentraut.org> In-Reply-To: From: Alexandra Wang Date: Fri, 22 Aug 2025 12:33:22 -0700 X-Gm-Features: Ac12FXzNBbOD0QRuUvrMDNpttOda5ecHmrcmBevxlJ0pGQOF1eCbL0clKGLfe9o Message-ID: Subject: Re: SQL:2023 JSON simplified accessor support To: jian he Cc: Nikita Malakhov , Vik Fearing , Mark Dilger , Matheus Alcantara , Peter Eisentraut , Andrew Dunstan , Nikita Glukhov , PostgreSQL Hackers , "David E. Wheeler" Content-Type: multipart/alternative; boundary="00000000000098c01a063cf949b9" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --00000000000098c01a063cf949b9 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Jian, On Fri, Aug 22, 2025 at 1:20=E2=80=AFAM jian he wrote: > On Thu, Aug 21, 2025 at 12:54=E2=80=AFPM Alexandra Wang > wrote: > > > > Hi Jian, > > > > Thanks for reviewing! I=E2=80=99ve attached v13, which addresses your > > feedback. > > > > hi. > in the context of v13-0001 and v13-0002. > > In transformIndirection: > while (subscripts) > { > Node *newresult =3D (Node *) =3D > transformContainerSubscripts(....&subscripts...) > } > > In transformContainerSubscripts: > sbsroutines->transform(sbsref, indirection, pstate, > isSlice, isAssignment); > /* > * Error out, if datatype failed to consume any indirection elements. > */ > if (list_length(*indirection) =3D=3D indirection_length) > { > } > As you can see from the above code pattern, "sbsroutines->transform" is > responsible for consuming the "indirection". > If it doesn=E2=80=99t consume it, the function should either return NULL = or raise > an > error. > But what happens if the elements consumed by "sbsroutines->transform" > are not contiguous? > > jsonb_subscript_transform below changes in v13-0002 > + if (!IsA(lfirst(idx), A_Indices)) > + break; > may cause elements consumed not contiguous. > > > diff --git a/src/backend/utils/adt/jsonbsubs.c > b/src/backend/utils/adt/jsonbsubs.c > index 8ad6aa1ad4f..a0d38a0fd80 100644 > --- a/src/backend/utils/adt/jsonbsubs.c > +++ b/src/backend/utils/adt/jsonbsubs.c > @@ -55,9 +55,14 @@ jsonb_subscript_transform(SubscriptingRef *sbsref, > */ > foreach(idx, *indirection) > { > - A_Indices *ai =3D lfirst_node(A_Indices, idx); > + A_Indices *ai; > Node *subExpr; > > + if (!IsA(lfirst(idx), A_Indices)) > + break; > + > + ai =3D lfirst_node(A_Indices, idx); > + > if (isSlice) > { > Node *expr =3D ai->uidx ? ai->uidx : ai->lidx; > @@ -160,7 +165,9 @@ jsonb_subscript_transform(SubscriptingRef *sbsref, > sbsref->refrestype =3D JSONBOID; > sbsref->reftypmod =3D -1; > > - *indirection =3D NIL; > + /* Remove processed elements */ > + if (upperIndexpr) > + *indirection =3D list_delete_first_n(*indirection, > list_length(upperIndexpr)); > } > > If the branch ``if (!IsA(lfirst(idx), A_Indices))`` is ever reached, then > ``*indirection =3D list_delete_first_n(*indirection, > list_length(upperIndexpr));`` > might produce an incorrect result? > > However, it appears that this branch is currently unreachable, > at least regress tests not coverage for > + if (!IsA(lfirst(idx), A_Indices)) > + break; > > Later patch we may have resolved this issue, but in the context of > 0001 and 0002, > this seem inconsistent? > I don=E2=80=99t understand the question. In the case of an unsupported Node type (not an Indices in patch 0001 or 0002), we break out of the loop to stop transforming the remaining subscripts. So there won=E2=80=99t be an= y =E2=80=98not contiguous=E2=80=99 indirection elements. Best, Alex --00000000000098c01a063cf949b9 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Jian,

On Fri, Aug 22, 2025 at= 1:20=E2=80=AFAM jian he <jian.universality@gmail.com> wrote:
On Thu, Aug 21, 2025 at 12:54=E2=80=AFPM Alexan= dra Wang
<alexa= ndra.wang.oss@gmail.com> wrote:
>
> Hi Jian,
>
> Thanks for reviewing! I=E2=80=99ve attached v13, which addresses your<= br> > feedback.
>

hi.
in the context of v13-0001 and v13-0002.

In transformIndirection:
while (subscripts)
{
=C2=A0 =C2=A0Node=C2=A0 =C2=A0 =C2=A0 =C2=A0*newresult =3D (Node *)=C2=A0 = =3D
transformContainerSubscripts(....&subscripts...)
}

In transformContainerSubscripts:
=C2=A0 =C2=A0 sbsroutines->transform(sbsref, indirection, pstate,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0isSlice, isAssignment);
=C2=A0 =C2=A0 /*
=C2=A0 =C2=A0 =C2=A0* Error out, if datatype failed to consume any indirect= ion elements.
=C2=A0 =C2=A0 =C2=A0*/
=C2=A0 =C2=A0 if (list_length(*indirection) =3D=3D indirection_length)
=C2=A0 =C2=A0 {
=C2=A0 =C2=A0 }
As you can see from the above code pattern, "sbsroutines->transform= " is
responsible for consuming the "indirection".
If it doesn=E2=80=99t consume it, the function should either return NULL or= raise an
error.
But what happens if the elements consumed by "sbsroutines->transfor= m"
are not contiguous?

jsonb_subscript_transform below changes in v13-0002
+ if (!IsA(lfirst(idx), A_Indices))
+ break;
may cause elements consumed not contiguous.


diff --git a/src/backend/utils/adt/jsonbsubs.c
b/src/backend/utils/adt/jsonbsubs.c
index 8ad6aa1ad4f..a0d38a0fd80 100644
--- a/src/backend/utils/adt/jsonbsubs.c
+++ b/src/backend/utils/adt/jsonbsubs.c
@@ -55,9 +55,14 @@ jsonb_subscript_transform(SubscriptingRef *sbsref,
=C2=A0 */
=C2=A0 foreach(idx, *indirection)
=C2=A0 {
- A_Indices=C2=A0 *ai =3D lfirst_node(A_Indices, idx);
+ A_Indices=C2=A0 *ai;
=C2=A0 Node=C2=A0 =C2=A0*subExpr;

+ if (!IsA(lfirst(idx), A_Indices))
+ break;
+
+ ai =3D lfirst_node(A_Indices, idx);
+
=C2=A0 if (isSlice)
=C2=A0 {
=C2=A0 Node=C2=A0 =C2=A0*expr =3D ai->uidx ? ai->uidx : ai->lidx;<= br> @@ -160,7 +165,9 @@ jsonb_subscript_transform(SubscriptingRef *sbsref,
=C2=A0 sbsref->refrestype =3D JSONBOID;
=C2=A0 sbsref->reftypmod =3D -1;

- *indirection =3D NIL;
+ /* Remove processed elements */
+ if (upperIndexpr)
+ *indirection =3D list_delete_first_n(*indirection, list_length(upperIndex= pr));
=C2=A0}

If the branch ``if (!IsA(lfirst(idx), A_Indices))`` is ever reached, then ``*indirection =3D list_delete_first_n(*indirection, list_length(upperIndex= pr));``
might produce an incorrect result?

However, it appears that this branch is currently unreachable,
at least regress tests not coverage for
+ if (!IsA(lfirst(idx), A_Indices))
+ break;

Later patch we may have resolved this issue, but in the context of
0001 and 0002,
this seem inconsistent?

I don=E2=80=99t= understand the question. In the case of an unsupported Node
type (not a= n Indices in patch 0001 or 0002), we break out of the loop
to stop trans= forming the remaining subscripts. So there won=E2=80=99t be any
=E2=80= =98not contiguous=E2=80=99 indirection elements.

Best,
Alex
--00000000000098c01a063cf949b9--