From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: "variable not found in subplan target list"
In-reply-to: <3463118.1680009420@sss.pgh.pa.us>
References: <20230328112248.6as34mlx5sr4kltg@alvherre.pgsql> <3460255.1680008201@sss.pgh.pa.us> <3463118.1680009420@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Tue, 28 Mar 2023 09:17:00 -0400"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <3508720.1680014730.0@sss.pgh.pa.us>
Date: Tue, 28 Mar 2023 10:46:29 -0400
Message-ID: <3508847.1680014789@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/3508847.1680014789%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3508720.1680014730.1@sss.pgh.pa.us>

I wrote:
> The planner is reducing the scan of target_parted to
> a dummy scan, as is reasonable, but it forgets to
> provide ctid as an output from that scan; then the
> parent join node is unhappy because it does have
> a ctid output.  So it looks like the problem is some
> shortcut we take while creating the dummy scan.

Oh, actually the problem is in distribute_row_identity_vars,
which is supposed to handle this case, but it thinks it doesn't
have to back-fill the rel's reltarget.  Wrong.  Now that I see
the problem, I wonder if we can't reproduce a similar symptom
without MERGE, which would mean that v14 has the issue too.

The attached seems to fix it, but I'm going to look for a
non-MERGE test case before pushing.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="fix-missing-ctid-variable.patch";
	charset="us-ascii"
Content-ID: <3508720.1680014730.2@sss.pgh.pa.us>
Content-Description: fix-missing-ctid-variable.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/optimizer/util/appendinfo.c b/src/backend/optimiz=
er/util/appendinfo.c
index 9d377385f1..c1b1557570 100644
--- a/src/backend/optimizer/util/appendinfo.c
+++ b/src/backend/optimizer/util/appendinfo.c
@@ -21,6 +21,7 @@
 #include "nodes/nodeFuncs.h"
 #include "optimizer/appendinfo.h"
 #include "optimizer/pathnode.h"
+#include "optimizer/planmain.h"
 #include "parser/parsetree.h"
 #include "utils/lsyscache.h"
 #include "utils/rel.h"
@@ -994,9 +995,10 @@ distribute_row_identity_vars(PlannerInfo *root)
 	 * certainly process no rows.  Handle this edge case by re-opening the t=
op
 	 * result relation and adding the row identity columns it would have use=
d,
 	 * as preprocess_targetlist() would have done if it weren't marked "inh"=
.
-	 * (This is a bit ugly, but it seems better to confine the ugliness and
-	 * extra cycles to this unusual corner case.)  We needn't worry about
-	 * fixing the rel's reltarget, as that won't affect the finished plan.
+	 * Then re-run build_base_rel_tlists() to ensure that the added columns
+	 * get propagated to the relation's reltarget.  (This is a bit ugly, but
+	 * it seems better to confine the ugliness and extra cycles to this
+	 * unusual corner case.)
 	 */
 	if (root->row_identity_vars =3D=3D NIL)
 	{
@@ -1006,6 +1008,8 @@ distribute_row_identity_vars(PlannerInfo *root)
 		add_row_identity_columns(root, result_relation,
 								 target_rte, target_relation);
 		table_close(target_relation, NoLock);
+		build_base_rel_tlists(root, root->processed_tlist);
+		/* There are no ROWID_VAR Vars in this case, so we're done. */
 		return;
 	}
 =


------- =_aaaaaaaaaa0--