Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1w3gQV-001S6H-0q for pgsql-bugs@arkaria.postgresql.org; Fri, 20 Mar 2026 20:26:35 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1w3gQT-008But-1p for pgsql-bugs@arkaria.postgresql.org; Fri, 20 Mar 2026 20:26:33 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1w3gQT-008Bul-10 for pgsql-bugs@lists.postgresql.org; Fri, 20 Mar 2026 20:26:33 +0000 Received: from sss.pgh.pa.us ([68.162.161.243]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.98.2) (envelope-from ) id 1w3gQN-00000000Elz-1oFf for pgsql-bugs@lists.postgresql.org; Fri, 20 Mar 2026 20:26:33 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.15.2/8.15.2) with ESMTP id 62KKQDf11777987; Fri, 20 Mar 2026 16:26:13 -0400 From: Tom Lane To: Alexander Korotkov cc: Andrei Lepikhov , Kirill Reshke , Tender Wang , Fujii Masao , 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: <1607553.1774017006@sss.pgh.pa.us> References: <19435-3cc1a87f291129f1@postgresql.org> <5a039d60-d! ! 13b-4cf0-a807-9c7269f06831@gmail.com> <1607553.1774017006@sss.pgh.pa.us> Comments: In-reply-to Tom Lane message dated "Fri, 20 Mar 2026 10:30:06 -0400" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <1777916.1774038349.0@sss.pgh.pa.us> Date: Fri, 20 Mar 2026 16:26:13 -0400 Message-ID: <1777986.1774038373@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <1777916.1774038349.1@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 ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="improve-ChangeVarNodesWalkExpression-comments.patch"; charset="us-ascii" Content-ID: <1777916.1774038349.2@sss.pgh.pa.us> Content-Description: improve-ChangeVarNodesWalkExpression-comments.patch Content-Transfer-Encoding: quoted-printable diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewr= iteManip.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_conte= xt *context) * value indicating if the given node should be skipped from further proc= essing * by ChangeVarNodes_walker. The callback is called only for expressions= and * other children nodes of a Query processed by a walker. Initial proces= sing - * 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_inde= x, 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) ------- =_aaaaaaaaaa0--