public inbox for pgsql-bugs@postgresql.org
help / color / mirror / Atom feedFrom: Alexander Korotkov <aekorotkov@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andrei Lepikhov <lepihov@gmail.com>
Cc: Kirill Reshke <reshkekirill@gmail.com>
Cc: Tender Wang <tndrwang@gmail.com>
Cc: Fujii Masao <masao.fujii@gmail.com>
Cc: ammmkilo@163.com
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #19435: Error: "No relation entry for relid 2" Triggered by Complex Join with Self-Referencing Tables
Date: Mon, 23 Mar 2026 23:28:45 +0200
Message-ID: <CAPpHfdvbjq342WTQ705Wmqhe8794pcp7wospz+WUJ2qB7vuOqA@mail.gmail.com> (raw)
In-Reply-To: <1777986.1774038373@sss.pgh.pa.us>
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>
Hi, Tom!
On Fri, Mar 20, 2026 at 10:26 PM 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
Attachments:
[application/octet-stream] further-improve-ChangeVarNodesWalkExpression-comments.patch (1.6K, 2-further-improve-ChangeVarNodesWalkExpression-comments.patch)
download | inline diff:
diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
index dc803a17037..0c6c63aebe5 100644
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -742,11 +742,17 @@ ChangeVarNodes(Node *node, int rt_index, int new_index, int sublevels_up)
*
* This is intended to be used by a callback that needs to recursively
* process subexpressions of some node being visited by an outer
- * ChangeVarNodesExtended call (not letting ChangeVarNodes_walker do that).
- * Hence, we invoke ChangeVarNodes_walker directly. This means that if
- * the passed Node is a Query node, it will be treated as a sub-Query,
- * so sublevels_up will be incremented immediately. Do not apply this
- * to a top-level Query node, or you'll likely get wrong results.
+ * ChangeVarNodesExtended call, instead of relying on ChangeVarNodes_walker's
+ * default recursion. We invoke ChangeVarNodes_walker directly rather than
+ * expression_tree_walker, because expression_tree_walker only visits child
+ * nodes and would fail to process the passed node itself --- for example,
+ * a bare Var node would not get its varno adjusted.
+ *
+ * Because this calls ChangeVarNodes_walker directly, if the passed node is
+ * a Query, it will be treated as a sub-Query: sublevels_up is incremented
+ * before recursing into it, and Query-level fields (resultRelation,
+ * mergeTargetRelation, rowMarks, etc.) will not be adjusted. Do not apply
+ * this to a top-level Query node; use ChangeVarNodesExtended for that.
*/
bool
ChangeVarNodesWalkExpression(Node *node, ChangeVarNodes_context *context)
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: pgsql-bugs@postgresql.org
Cc: aekorotkov@gmail.com, tgl@sss.pgh.pa.us, lepihov@gmail.com, reshkekirill@gmail.com, tndrwang@gmail.com, masao.fujii@gmail.com, ammmkilo@163.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #19435: Error: "No relation entry for relid 2" Triggered by Complex Join with Self-Referencing Tables
In-Reply-To: <CAPpHfdvbjq342WTQ705Wmqhe8794pcp7wospz+WUJ2qB7vuOqA@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