public inbox for pgsql-bugs@postgresql.org
help / color / mirror / Atom feedFrom: Tom Lane <tgl@sss.pgh.pa.us>
To: Alexander Korotkov <aekorotkov@gmail.com>
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: Fri, 20 Mar 2026 16:26:13 -0400
Message-ID: <1777986.1774038373@sss.pgh.pa.us> (raw)
In-Reply-To: <1607553.1774017006@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>
<5a039d60-d! ! 13b-4cf0-a807-9c7269f06831@gmail.com>
<CAPpHfdsyNYEbjjLdsa8i8Ds-5=4pFif1+uCHn3vwzx2Pq5y29A@mail.gmail.com>
<CAPpHfdsrmAg+aqpjAF4Fdp2c59-dFmwBuNLhNqrxzTguiAKf=w@mail.gmail.com>
<1607553.1774017006@sss.pgh.pa.us>
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.)
regards, tom lane
Attachments:
[text/x-diff] improve-ChangeVarNodesWalkExpression-comments.patch (1.6K, 2-improve-ChangeVarNodesWalkExpression-comments.patch)
download | inline diff:
diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
index 7249ffbfb36..dc803a17037 100644
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -672,7 +672,7 @@ ChangeVarNodes_walker(Node *node, ChangeVarNodes_context *context)
* value indicating if the given node should be skipped from further processing
* by ChangeVarNodes_walker. The callback is called only for expressions and
* other children nodes of a Query processed by a walker. Initial processing
- * of the root Query doesn't involve the callback.
+ * of the root Query node doesn't invoke the callback.
*/
void
ChangeVarNodesExtended(Node *node, int rt_index, int new_index,
@@ -737,9 +737,16 @@ ChangeVarNodes(Node *node, int rt_index, int new_index, int sublevels_up)
}
/*
- * ChangeVarNodesWalkExpression - process expression within the custom
- * callback provided to the
- * ChangeVarNodesExtended.
+ * ChangeVarNodesWalkExpression - process subexpression within a callback
+ * function passed to ChangeVarNodesExtended.
+ *
+ * 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.
*/
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: tgl@sss.pgh.pa.us, aekorotkov@gmail.com, 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: <1777986.1774038373@sss.pgh.pa.us>
* 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