MIME-Version: 1.0
References: <tencent_4CC6316F02DECA61ACCF22F933FEA5C12806@qq.com>
 <9EE922BD-29B2-40AF-A50E-03D7E89C10DB@gmail.com>
 <CA+HiwqGJ_hJneTeOVbaJs=ULoCjfgchjgLu8i_CEai4FX+8Vyw@mail.gmail.com>
In-Reply-To: 
 <CA+HiwqGJ_hJneTeOVbaJs=ULoCjfgchjgLu8i_CEai4FX+8Vyw@mail.gmail.com>
From: Amit Langote <amitlangote09@gmail.com>
Date: Thu, 16 Apr 2026 14:17:22 +0900
Message-ID: 
 <CA+HiwqG3TDcUF3kGhK=RYsF2RofpGm0RwDDH+87H-iLQo8xP9g@mail.gmail.com>
Subject: Re: [PATCH] Fix wrong comment in JsonTablePlanJoinNextRow()
To: Chao Li <li.evan.chao@gmail.com>
Cc: =?UTF-8?B?6IOh5Lyg5paH?= <463945512@qq.com>,
	pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CA%2BHiwqG3TDcUF3kGhK%3DRYsF2RofpGm0RwDDH%2B87H-iLQo8xP9g%40mail.gmail.com>
Precedence: bulk

On Thu, Apr 16, 2026 at 12:03=E2=80=AFPM Amit Langote <amitlangote09@gmail.=
com> wrote:
> On Thu, Apr 16, 2026 at 11:05=E2=80=AFAM Chao Li <li.evan.chao@gmail.com>=
 wrote:
> > > On Apr 15, 2026, at 16:28, =E8=83=A1=E4=BC=A0=E6=96=87 <463945512@qq.=
com> wrote:
> > >
> > > Hi,
> > > Found a misleading comment in JsonTablePlanJoinNextRow() while readin=
g
> > > the JSON_TABLE execution code.
> > > The function returns false when both siblings are exhausted (meaning =
no
> > > more rows), but the comment says "there are more rows" =E2=80=94 the =
exact
> > > opposite of what's happening. The code itself is correct.
> > > if (!JsonTablePlanNextRow(planstate->right))
> > > {
> > > /* Right sibling ran out of row, so there are more rows. */ /* wrong =
*/
> > > return false;
> > > }
> > > A reader might reasonably treat this as a bug and flip the return val=
ue,
> > > which would cause JSON_TABLE UNION plans to loop indefinitely.
> > > Patch attached.
>
> Thanks for the report and the patch.
>
> > The fix looks correct to me. I guess =E2=80=9Cno=E2=80=9D was just unin=
tentionally missed.
> >
> > A small comment, I just feel =E2=80=9Ctoo=E2=80=9D you newly added migh=
t not be needed.
>
> Actually, I'd keep it, because it ties the comment to the left-sibling
> check just above.
>
> Will push the attached down to v17.

Done.

--=20
Thanks, Amit Langote