| From: | Amit Langote <amitlangote09(at)gmail(dot)com> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
| Cc: | 胡传文 <463945512(at)qq(dot)com>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: [PATCH] Fix wrong comment in JsonTablePlanJoinNextRow() |
| Date: | 2026-04-16 05:17:22 |
| Message-ID: | CA+HiwqG3TDcUF3kGhK=RYsF2RofpGm0RwDDH+87H-iLQo8xP9g@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Apr 16, 2026 at 12:03 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> On Thu, Apr 16, 2026 at 11:05 AM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
> > > On Apr 15, 2026, at 16:28, 胡传文 <463945512(at)qq(dot)com> 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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Xiaopeng Wang | 2026-04-16 05:27:13 | Re: Use XLogRecPtrIsValid() instead of negated XLogRecPtrIsInvalid |
| Previous Message | Amit Langote | 2026-04-16 05:17:08 | Re: pg_overexplain produces invalid JSON with RANGE_TABLE option |