MIME-Version: 1.0
References: <tencent_4CC6316F02DECA61ACCF22F933FEA5C12806@qq.com>
 <9EE922BD-29B2-40AF-A50E-03D7E89C10DB@gmail.com>
In-Reply-To: <9EE922BD-29B2-40AF-A50E-03D7E89C10DB@gmail.com>
From: Amit Langote <amitlangote09@gmail.com>
Date: Thu, 16 Apr 2026 12:03:22 +0900
Message-ID: 
 <CA+HiwqGJ_hJneTeOVbaJs=ULoCjfgchjgLu8i_CEai4FX+8Vyw@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: multipart/mixed; boundary="000000000000522593064f8b1496"
Archived-At: 
 <https://www.postgresql.org/message-id/CA%2BHiwqGJ_hJneTeOVbaJs%3DULoCjfgchjgLu8i_CEai4FX%2B8Vyw%40mail.gmail.com>
Precedence: bulk

--000000000000522593064f8b1496
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi,

On Thu, Apr 16, 2026 at 11:05=E2=80=AFAM Chao Li <li.evan.chao@gmail.com> w=
rote:
> > On Apr 15, 2026, at 16:28, =E8=83=A1=E4=BC=A0=E6=96=87 <463945512@qq.co=
m> 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" =E2=80=94 the ex=
act
> > 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 =E2=80=9Cno=E2=80=9D was just uninte=
ntionally missed.
>
> A small comment, I just feel =E2=80=9Ctoo=E2=80=9D 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.

--=20
Thanks, Amit Langote

--000000000000522593064f8b1496
Content-Type: application/octet-stream;
	name="v1-0001-Fix-incorrect-comment-in-JsonTablePlanJoinNextRow.patch"
Content-Disposition: attachment;
	filename="v1-0001-Fix-incorrect-comment-in-JsonTablePlanJoinNextRow.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_mo0w7tjs0>
X-Attachment-Id: f_mo0w7tjs0

RnJvbSBiMDY3NzE3OWUxMTZmZDkxYmI1ZDgzYTI0YjUxN2U4NWM1NDQ2YTkxIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBBbWl0IExhbmdvdGUgPGFtaXRsYW5AcG9zdGdyZXNxbC5vcmc+
CkRhdGU6IFRodSwgMTYgQXByIDIwMjYgMTE6NTI6MTYgKzA5MDAKU3ViamVjdDogW1BBVENIIHYx
XSBGaXggaW5jb3JyZWN0IGNvbW1lbnQgaW4gSnNvblRhYmxlUGxhbkpvaW5OZXh0Um93KCkKClRo
ZSBjb21tZW50IG9uIHRoZSByZXR1cm4tZmFsc2UgcGF0aCB3aGVuIGJvdGggVU5JT04gc2libGlu
Z3MgYXJlCmV4aGF1c3RlZCBzYWlkICJ0aGVyZSBhcmUgbW9yZSByb3dzLCIgd2hpY2ggaXMgdGhl
IG9wcG9zaXRlIG9mIHdoYXQKdGhlIGNvZGUgZG9lcy4gVGhlIGNvZGUgaXRzZWxmIGlzIGNvcnJl
Y3QsIHJldHVybmluZyBmYWxzZSB0byBzaWduYWwKbm8gbW9yZSByb3dzLCBidXQgdGhlIG1pc2xl
YWRpbmcgY29tbWVudCBjb3VsZCB0ZW1wdCBhIHJlYWRlciBpbnRvCiJmaXhpbmciIHRoZSByZXR1
cm4gdmFsdWUsIHdoaWNoIHdvdWxkIGNhdXNlIFVOSU9OIHBsYW5zIHRvIGxvb3AKaW5kZWZpbml0
ZWx5LgoKQmFjay1wYXRjaCB0byAxNywgd2hlcmUgSlNPTl9UQUJMRSB3YXMgaW50cm9kdWNlZC4K
CkF1dGhvcjogQ2h1YW53ZW4gSHUgPDQ2Mzk0NTUxMkBxcS5jb20+ClJldmlld2VkLWJ5OiBDaGFv
IExpIDxsaS5ldmFuLmNoYW9AZ21haWwuY29tPgpEaXNjdXNzaW9uOiBodHRwczovL3Bvc3Rnci5l
cy9tL3RlbmNlbnRfNENDNjMxNkYwMkRFQ0E2MUFDQ0YyMkY5MzNGRUE1QzEyODA2QHFxLmNvbQpC
YWNrcGF0Y2gtdGhyb3VnaDogMTcKLS0tCiBzcmMvYmFja2VuZC91dGlscy9hZHQvanNvbnBhdGhf
ZXhlYy5jIHwgMiArLQogMSBmaWxlIGNoYW5nZWQsIDEgaW5zZXJ0aW9uKCspLCAxIGRlbGV0aW9u
KC0pCgpkaWZmIC0tZ2l0IGEvc3JjL2JhY2tlbmQvdXRpbHMvYWR0L2pzb25wYXRoX2V4ZWMuYyBi
L3NyYy9iYWNrZW5kL3V0aWxzL2FkdC9qc29ucGF0aF9leGVjLmMKaW5kZXggNzcwODQwYTA2MTEu
LjBlYzliNGRmMmVmIDEwMDY0NAotLS0gYS9zcmMvYmFja2VuZC91dGlscy9hZHQvanNvbnBhdGhf
ZXhlYy5jCisrKyBiL3NyYy9iYWNrZW5kL3V0aWxzL2FkdC9qc29ucGF0aF9leGVjLmMKQEAgLTQ3
MjksNyArNDcyOSw3IEBAIEpzb25UYWJsZVBsYW5Kb2luTmV4dFJvdyhKc29uVGFibGVQbGFuU3Rh
dGUgKnBsYW5zdGF0ZSkKIAkJICovCiAJCWlmICghSnNvblRhYmxlUGxhbk5leHRSb3cocGxhbnN0
YXRlLT5yaWdodCkpCiAJCXsKLQkJCS8qIFJpZ2h0IHNpYmxpbmcgcmFuIG91dCBvZiByb3csIHNv
IHRoZXJlIGFyZSBtb3JlIHJvd3MuICovCisJCQkvKiBSaWdodCBzaWJsaW5nIHJhbiBvdXQgb2Yg
cm93cyB0b28sIHNvIHRoZXJlIGFyZSBubyBtb3JlIHJvd3MuICovCiAJCQlyZXR1cm4gZmFsc2U7
CiAJCX0KIAl9Ci0tIAoyLjQ3LjMKCg==
--000000000000522593064f8b1496--