MIME-Version: 1.0
References: <19435-3cc1a87f291129f1@postgresql.org> <CAHGQGwEEHFnH8DsZbwxdZeiHozm2LCRTLSgL8Qn=5MoN4450ZA@mail.gmail.com>
 <CAPpHfdsDQhDqvqi+-UxvyYRP72ASkdWzCn43e2Hj6WsY15opvA@mail.gmail.com>
 <CAHewXNmU3E0oXLgvoOtXw7kfTiAZsKu8La_hQ=pyScgxvP45iw@mail.gmail.com>
 <CAHewXN=LjuWz3PcyhjdbJAyo+Zs9MisPDRYnSZBUy4PMeKi+zA@mail.gmail.com>
 <CALdSSPj1kTTQvmV3H3HMf5P3um8ybxoH3DaTPm+XgdYAur1Q4A@mail.gmail.com>
 <CAHewXNndByMu3S+_h4LLDkXA5qrO1s=s-CE8HqUtc9vTA9yrjg@mail.gmail.com>
 <CAPpHfdv6gzSTXHJxYSgB8sULadXM4wvhgoQODaOxYCJfagKNPw@mail.gmail.com>
 <CAHewXN=7kDJjUcgEm+6qhaKOXuqzvhRqAAKdafNCRgn0yH7BGg@mail.gmail.com>
 <CAHewXNm5OOREJ8wZ1cLJdQz7O1aQ0E1RBB55S6O138K8vBdc9g@mail.gmail.com>
 <CAPpHfducqLJ=o3LkoPKGfZJVQuuei+P=2oUF6hX6rzHTZSxoyA@mail.gmail.com>
 <a78fe5d4-e6b8-4b3c-9cfd-135edbb68e4c@gmail.com> <CAPpHfduTWFCHaK8U7bDfYid5pjVA=FHG1b0nTEMFqFKHebGJxQ@mail.gmail.com>
 <a498f5b8-2f17-4ee0-b021-63ff9829b45b@gmail.com> <CALdSSPhpUdY7-5Zg38oS1uRtu5iTFzdo0R7Z2YZD603M9RpJxg@mail.gmail.com>
 <CAPpHfdsyNYEbjjLdsa8i8Ds-5=4pFif1+uCHn3vwzx2Pq5y29A@mail.gmail.com>
 <CAPpHfdsrmAg+aqpjAF4Fdp2c59-dFmwBuNLhNqrxzTguiAKf=w@mail.gmail.com>
 <1607553.1774017006@sss.pgh.pa.us> <1777986.1774038373@sss.pgh.pa.us>
In-Reply-To: <1777986.1774038373@sss.pgh.pa.us>
From: Alexander Korotkov <aekorotkov@gmail.com>
Date: Mon, 23 Mar 2026 23:28:45 +0200
Message-ID: <CAPpHfdvbjq342WTQ705Wmqhe8794pcp7wospz+WUJ2qB7vuOqA@mail.gmail.com>
Subject: Re: BUG #19435: Error: "No relation entry for relid 2" Triggered by
 Complex Join with Self-Referencing Tables
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andrei Lepikhov <lepihov@gmail.com>, Kirill Reshke <reshkekirill@gmail.com>, 
	Tender Wang <tndrwang@gmail.com>, Fujii Masao <masao.fujii@gmail.com>, ammmkilo@163.com, 
	pgsql-bugs@lists.postgresql.org
Content-Type: multipart/mixed; boundary="0000000000000cb026064db7b913"
Archived-At: <https://www.postgresql.org/message-id/CAPpHfdvbjq342WTQ705Wmqhe8794pcp7wospz%2BWUJ2qB7vuOqA%40mail.gmail.com>
Precedence: bulk

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

Hi, Tom!

On Fri, Mar 20, 2026 at 10:26=E2=80=AFPM Tom Lane <tgl@sss.pgh.pa.us> wrote=
:
>
> I wrote:
> > At the very least we need to add comments, but I wonder if we
> > don't actually need an Assert that ChangeVarNodesWalkExpression
> > is not invoked directly on a Query.  It would have done the
> > right thing before this patch, but now it won't.  That's an
> > okay tradeoff for fixing the bare-Var case, but not documenting
> > what you did is not okay.
>
> After further contemplation I've decided that an Assert would be
> wrong, because it's not impossible that a callback would want
> to invoke this on a sub-Query --- for instance, if it wanted to
> short-circuit ChangeVarNodes's processing of a SubLink node,
> it would need to do that.  The key point is that if we do see a
> Query node here, we will treat it as a sub-query not a top-level
> query, which also justifies skipping the work that
> ChangeVarNodesExtended does on a top-level Query.  So we just
> need a comment explaining that.  I'm thinking about the attached.
>
> (BTW, by this reasoning the previous implementation of
> ChangeVarNodesWalkExpression was doubly wrong, since it would
> have done the wrong thing at a Query node as well as a Var node.)

Thank you so much for caring about this.  I agree that this kind of
changes should go with proper comments.

Please, consider my additions to the comment.  They explain why we use
ChangeVarNodes_walker() instead of expression_tree_walker(), and gives
a bit more details about difference in processing of top-level Query
and subquery.

------
Regards,
Alexander Korotkov
Supabase

--0000000000000cb026064db7b913
Content-Type: application/octet-stream; 
	name="further-improve-ChangeVarNodesWalkExpression-comments.patch"
Content-Disposition: attachment; 
	filename="further-improve-ChangeVarNodesWalkExpression-comments.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_mn3p4g490>
X-Attachment-Id: f_mn3p4g490

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3Jld3JpdGUvcmV3cml0ZU1hbmlwLmMgYi9zcmMvYmFj
a2VuZC9yZXdyaXRlL3Jld3JpdGVNYW5pcC5jCmluZGV4IGRjODAzYTE3MDM3Li4wYzZjNjNhZWJl
NSAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvcmV3cml0ZS9yZXdyaXRlTWFuaXAuYworKysgYi9z
cmMvYmFja2VuZC9yZXdyaXRlL3Jld3JpdGVNYW5pcC5jCkBAIC03NDIsMTEgKzc0MiwxNyBAQCBD
aGFuZ2VWYXJOb2RlcyhOb2RlICpub2RlLCBpbnQgcnRfaW5kZXgsIGludCBuZXdfaW5kZXgsIGlu
dCBzdWJsZXZlbHNfdXApCiAgKgogICogVGhpcyBpcyBpbnRlbmRlZCB0byBiZSB1c2VkIGJ5IGEg
Y2FsbGJhY2sgdGhhdCBuZWVkcyB0byByZWN1cnNpdmVseQogICogcHJvY2VzcyBzdWJleHByZXNz
aW9ucyBvZiBzb21lIG5vZGUgYmVpbmcgdmlzaXRlZCBieSBhbiBvdXRlcgotICogQ2hhbmdlVmFy
Tm9kZXNFeHRlbmRlZCBjYWxsIChub3QgbGV0dGluZyBDaGFuZ2VWYXJOb2Rlc193YWxrZXIgZG8g
dGhhdCkuCi0gKiBIZW5jZSwgd2UgaW52b2tlIENoYW5nZVZhck5vZGVzX3dhbGtlciBkaXJlY3Rs
eS4gIFRoaXMgbWVhbnMgdGhhdCBpZgotICogdGhlIHBhc3NlZCBOb2RlIGlzIGEgUXVlcnkgbm9k
ZSwgaXQgd2lsbCBiZSB0cmVhdGVkIGFzIGEgc3ViLVF1ZXJ5LAotICogc28gc3VibGV2ZWxzX3Vw
IHdpbGwgYmUgaW5jcmVtZW50ZWQgaW1tZWRpYXRlbHkuICBEbyBub3QgYXBwbHkgdGhpcwotICog
dG8gYSB0b3AtbGV2ZWwgUXVlcnkgbm9kZSwgb3IgeW91J2xsIGxpa2VseSBnZXQgd3JvbmcgcmVz
dWx0cy4KKyAqIENoYW5nZVZhck5vZGVzRXh0ZW5kZWQgY2FsbCwgaW5zdGVhZCBvZiByZWx5aW5n
IG9uIENoYW5nZVZhck5vZGVzX3dhbGtlcidzCisgKiBkZWZhdWx0IHJlY3Vyc2lvbi4gIFdlIGlu
dm9rZSBDaGFuZ2VWYXJOb2Rlc193YWxrZXIgZGlyZWN0bHkgcmF0aGVyIHRoYW4KKyAqIGV4cHJl
c3Npb25fdHJlZV93YWxrZXIsIGJlY2F1c2UgZXhwcmVzc2lvbl90cmVlX3dhbGtlciBvbmx5IHZp
c2l0cyBjaGlsZAorICogbm9kZXMgYW5kIHdvdWxkIGZhaWwgdG8gcHJvY2VzcyB0aGUgcGFzc2Vk
IG5vZGUgaXRzZWxmIC0tLSBmb3IgZXhhbXBsZSwKKyAqIGEgYmFyZSBWYXIgbm9kZSB3b3VsZCBu
b3QgZ2V0IGl0cyB2YXJubyBhZGp1c3RlZC4KKyAqCisgKiBCZWNhdXNlIHRoaXMgY2FsbHMgQ2hh
bmdlVmFyTm9kZXNfd2Fsa2VyIGRpcmVjdGx5LCBpZiB0aGUgcGFzc2VkIG5vZGUgaXMKKyAqIGEg
UXVlcnksIGl0IHdpbGwgYmUgdHJlYXRlZCBhcyBhIHN1Yi1RdWVyeTogc3VibGV2ZWxzX3VwIGlz
IGluY3JlbWVudGVkCisgKiBiZWZvcmUgcmVjdXJzaW5nIGludG8gaXQsIGFuZCBRdWVyeS1sZXZl
bCBmaWVsZHMgKHJlc3VsdFJlbGF0aW9uLAorICogbWVyZ2VUYXJnZXRSZWxhdGlvbiwgcm93TWFy
a3MsIGV0Yy4pIHdpbGwgbm90IGJlIGFkanVzdGVkLiAgRG8gbm90IGFwcGx5CisgKiB0aGlzIHRv
IGEgdG9wLWxldmVsIFF1ZXJ5IG5vZGU7IHVzZSBDaGFuZ2VWYXJOb2Rlc0V4dGVuZGVkIGZvciB0
aGF0LgogICovCiBib29sCiBDaGFuZ2VWYXJOb2Rlc1dhbGtFeHByZXNzaW9uKE5vZGUgKm5vZGUs
IENoYW5nZVZhck5vZGVzX2NvbnRleHQgKmNvbnRleHQpCg==
--0000000000000cb026064db7b913--