public inbox for [email protected]  

help / color / mirror / Atom feed

From: Amit Langote <[email protected]>
To: Chao Li <[email protected]>
Cc: 胡传文 <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: [PATCH] Fix wrong comment in JsonTablePlanJoinNextRow()
Date: Thu, 16 Apr 2026 14:17:22 +0900
Message-ID: <CA+HiwqG3TDcUF3kGhK=RYsF2RofpGm0RwDDH+87H-iLQo8xP9g@mail.gmail.com> (raw)
In-Reply-To: <CA+HiwqGJ_hJneTeOVbaJs=ULoCjfgchjgLu8i_CEai4FX+8Vyw@mail.gmail.com>
References: <[email protected]>
	<[email protected]>
	<CA+HiwqGJ_hJneTeOVbaJs=ULoCjfgchjgLu8i_CEai4FX+8Vyw@mail.gmail.com>

On Thu, Apr 16, 2026 at 12:03 PM Amit Langote <[email protected]> wrote:
> On Thu, Apr 16, 2026 at 11:05 AM Chao Li <[email protected]> wrote:
> > > On Apr 15, 2026, at 16:28, 胡传文 <[email protected]> wrote:
> > >
> > > Hi,
> > > Found a misleading comment in JsonTablePlanJoinNextRow() while reading
> > > 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" — 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 value,
> > > 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 “no” was just unintentionally missed.
> >
> > A small comment, I just feel “too” you newly added might 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.

-- 
Thanks, Amit Langote

view thread (4+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] Fix wrong comment in JsonTablePlanJoinNextRow()
  In-Reply-To: <CA+HiwqG3TDcUF3kGhK=RYsF2RofpGm0RwDDH+87H-iLQo8xP9g@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox