v3-0006-fix-FDWs.patch

commit 9741326c32bebe41fcb1febbcc9289328eec2dbb
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date:   Mon Aug 1 15:41:56 2022 -0400

    Teach FDWs about base-plus-outer-join relids.
    
    Conversion of the planner to include OJ relids in join relids
    affects FDWs that want to plan foreign joins.  They *must* follow
    suit when labeling foreign joins in order to match with the core
    planner, but for many purposes (if postgres_fdw is any guide)
    they'd prefer to consider only base relations within the join.
    To support both requirements, redefine ForeignScan.fs_relids as
    base+OJ relids, and add a new field fs_base_relids that's set up
    by the core planner.
    
    Another way we could do this is to keep fs_relids as just base
    relids and make the new field be the one with OJ relids added.
    While that would be more backwards-compatible in some sense,
    it would be inconsistent with the naming used in the core planner,
    and I think that it might allow some types of bugs to escape
    immediate detection.
    
    postgres_fdw also has one place where it needs to ignore varnullingrels
    while matching Vars, similarly to the unfinished work in setrefs.c.
    (That requirement will only affect join-planning FDWs, too, since
    Vars seen at a base relation scan should never have any varnullingrels.)

diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index a9766f9734..a187cd08fa 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -3952,7 +3952,17 @@ get_relation_column_alias_ids(Var *node, RelOptInfo *foreignrel,
 	i = 1;
 	foreach(lc, foreignrel->reltarget->exprs)
 	{
-		if (equal(lfirst(lc), (Node *) node))
+		Var		   *tlvar = (Var *) lfirst(lc);
+
+		/*
+		 * As in setrefs.c, we match only on varno/varattno.  Ideally there
+		 * would be some cross-check on varnullingrels, but it's unclear what
+		 * to do exactly; we don't have enough context to know what that value
+		 * should be.
+		 */
+		if (IsA(tlvar, Var) &&
+			tlvar->varno == node->varno &&
+			tlvar->varattno == node->varattno)
 		{
 			*colno = i;
 			return;
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 048db542d3..08d5042ffc 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -1513,13 +1513,13 @@ postgresBeginForeignScan(ForeignScanState *node, int eflags)
 	/*
 	 * Identify which user to do the remote access as.  This should match what
 	 * ExecCheckRTEPerms() does.  In case of a join or aggregate, use the
-	 * lowest-numbered member RTE as a representative; we would get the same
-	 * result from any.
+	 * lowest-numbered member base RTE as a representative; we would get the
+	 * same result from any.
 	 */
 	if (fsplan->scan.scanrelid > 0)
 		rtindex = fsplan->scan.scanrelid;
 	else
-		rtindex = bms_next_member(fsplan->fs_relids, -1);
+		rtindex = bms_next_member(fsplan->fs_base_relids, -1);
 	rte = exec_rt_fetch(rtindex, estate);
 	userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
 
@@ -2405,7 +2405,7 @@ find_modifytable_subplan(PlannerInfo *root,
 	{
 		ForeignScan *fscan = (ForeignScan *) subplan;
 
-		if (bms_is_member(rtindex, fscan->fs_relids))
+		if (bms_is_member(rtindex, fscan->fs_base_relids))
 			return fscan;
 	}
 
@@ -2831,8 +2831,8 @@ postgresExplainForeignScan(ForeignScanState *node, ExplainState *es)
 		 * that setrefs.c won't update the string when flattening the
 		 * rangetable.  To find out what rtoffset was applied, identify the
 		 * minimum RT index appearing in the string and compare it to the
-		 * minimum member of plan->fs_relids.  (We expect all the relids in
-		 * the join will have been offset by the same amount; the Asserts
+		 * minimum member of plan->fs_base_relids.  (We expect all the relids
+		 * in the join will have been offset by the same amount; the Asserts
 		 * below should catch it if that ever changes.)
 		 */
 		minrti = INT_MAX;
@@ -2849,7 +2849,7 @@ postgresExplainForeignScan(ForeignScanState *node, ExplainState *es)
 			else
 				ptr++;
 		}
-		rtoffset = bms_next_member(plan->fs_relids, -1) - minrti;
+		rtoffset = bms_next_member(plan->fs_base_relids, -1) - minrti;
 
 		/* Now we can translate the string */
 		relations = makeStringInfo();
@@ -2864,7 +2864,7 @@ postgresExplainForeignScan(ForeignScanState *node, ExplainState *es)
 				char	   *refname;
 
 				rti += rtoffset;
-				Assert(bms_is_member(rti, plan->fs_relids));
+				Assert(bms_is_member(rti, plan->fs_base_relids));
 				rte = rt_fetch(rti, es->rtable);
 				Assert(rte->rtekind == RTE_RELATION);
 				/* This logic should agree with explain.c's ExplainTargetRel */
diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml
index d0b5951019..329affa30b 100644
--- a/doc/src/sgml/fdwhandler.sgml
+++ b/doc/src/sgml/fdwhandler.sgml
@@ -351,6 +351,17 @@ GetForeignJoinPaths(PlannerInfo *root,
      it will supply at run time in the tuples it returns.
     </para>
 
+    <note>
+     <para>
+      Beginning with <productname>PostgreSQL</productname> 16,
+      <structfield>fs_relids</structfield> includes the rangetable indexes
+      of outer joins, if any were involved in this join.  The new field
+      <structfield>fs_base_relids</structfield> includes only base
+      relation indexes, and thus
+      mimics <structfield>fs_relids</structfield>'s old semantics.
+     </para>
+    </note>
+
     <para>
      See <xref linkend="fdw-planning"/> for additional information.
     </para>
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index e29c2ae206..ded37cb2e9 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -1114,7 +1114,7 @@ ExplainPreScanNode(PlanState *planstate, Bitmapset **rels_used)
 			break;
 		case T_ForeignScan:
 			*rels_used = bms_add_members(*rels_used,
-										 ((ForeignScan *) plan)->fs_relids);
+										 ((ForeignScan *) plan)->fs_base_relids);
 			break;
 		case T_CustomScan:
 			*rels_used = bms_add_members(*rels_used,
diff --git a/src/backend/executor/execScan.c b/src/backend/executor/execScan.c
index 043bb83f55..2b37266b6a 100644
--- a/src/backend/executor/execScan.c
+++ b/src/backend/executor/execScan.c
@@ -325,7 +325,7 @@ ExecScanReScan(ScanState *node)
 			 * all of them.
 			 */
 			if (IsA(node->ps.plan, ForeignScan))
-				relids = ((ForeignScan *) node->ps.plan)->fs_relids;
+				relids = ((ForeignScan *) node->ps.plan)->fs_base_relids;
 			else if (IsA(node->ps.plan, CustomScan))
 				relids = ((CustomScan *) node->ps.plan)->custom_relids;
 			else
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index e37f2933eb..bd9af88d55 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -29,6 +29,7 @@
 #include "optimizer/cost.h"
 #include "optimizer/optimizer.h"
 #include "optimizer/paramassign.h"
+#include "optimizer/pathnode.h"
 #include "optimizer/paths.h"
 #include "optimizer/placeholder.h"
 #include "optimizer/plancat.h"
@@ -4105,6 +4106,8 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
 	Index		scan_relid = rel->relid;
 	Oid			rel_oid = InvalidOid;
 	Plan	   *outer_plan = NULL;
+	Relids		fs_base_relids;
+	int			rtindex;
 
 	Assert(rel->fdwroutine != NULL);
 
@@ -4153,14 +4156,28 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
 
 	/*
 	 * Likewise, copy the relids that are represented by this foreign scan. An
-	 * upper rel doesn't have relids set, but it covers all the base relations
-	 * participating in the underlying scan, so use root's all_baserels.
+	 * upper rel doesn't have relids set, but it covers all the relations
+	 * participating in the underlying scan/join, so use root->all_query_rels.
 	 */
 	if (rel->reloptkind == RELOPT_UPPER_REL)
-		scan_plan->fs_relids = root->all_baserels;
+		scan_plan->fs_relids = root->all_query_rels;
 	else
 		scan_plan->fs_relids = best_path->path.parent->relids;
 
+	/*
+	 * Join relid sets include relevant outer joins, but FDWs may need to know
+	 * which are the included base rels.  That's a bit tedious to get without
+	 * access to the plan-time data structures, so compute it here.
+	 */
+	fs_base_relids = NULL;
+	rtindex = -1;
+	while ((rtindex = bms_next_member(scan_plan->fs_relids, rtindex)) >= 0)
+	{
+		if (find_base_rel_ignore_join(root, rtindex) != NULL)
+			fs_base_relids = bms_add_member(fs_base_relids, rtindex);
+	}
+	scan_plan->fs_base_relids = fs_base_relids;
+
 	/*
 	 * If this is a foreign join, and to make it valid to push down we had to
 	 * assume that the current user is the same as some user explicitly named
@@ -5805,8 +5822,9 @@ make_foreignscan(List *qptlist,
 	node->fdw_private = fdw_private;
 	node->fdw_scan_tlist = fdw_scan_tlist;
 	node->fdw_recheck_quals = fdw_recheck_quals;
-	/* fs_relids will be filled in by create_foreignscan_plan */
+	/* fs_relids, fs_base_relids will be filled by create_foreignscan_plan */
 	node->fs_relids = NULL;
+	node->fs_base_relids = NULL;
 	/* fsSystemCol will be filled in by create_foreignscan_plan */
 	node->fsSystemCol = false;
 
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index a1827d113d..1a047ddda2 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -1533,6 +1533,7 @@ set_foreignscan_references(PlannerInfo *root,
 	}
 
 	fscan->fs_relids = offset_relid_set(fscan->fs_relids, rtoffset);
+	fscan->fs_base_relids = offset_relid_set(fscan->fs_base_relids, rtoffset);
 
 	/* Adjust resultRelation if it's valid */
 	if (fscan->resultRelation > 0)
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index dca2a21e7a..7e98d0b7a3 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -688,6 +688,7 @@ typedef struct WorkTableScan
  * When the plan node represents a foreign join, scan.scanrelid is zero and
  * fs_relids must be consulted to identify the join relation.  (fs_relids
  * is valid for simple scans as well, but will always match scan.scanrelid.)
+ * fs_relids includes outer joins; fs_base_relids does not.
  *
  * If the FDW's PlanDirectModify() callback decides to repurpose a ForeignScan
  * node to perform the UPDATE or DELETE operation directly in the remote
@@ -707,7 +708,8 @@ typedef struct ForeignScan
 	List	   *fdw_private;	/* private data for FDW */
 	List	   *fdw_scan_tlist; /* optional tlist describing scan tuple */
 	List	   *fdw_recheck_quals;	/* original quals not in scan.plan.qual */
-	Bitmapset  *fs_relids;		/* RTIs generated by this scan */
+	Bitmapset  *fs_relids;		/* base+OJ RTIs generated by this scan */
+	Bitmapset  *fs_base_relids; /* base RTIs generated by this scan */
 	bool		fsSystemCol;	/* true if any "system column" is needed */
 } ForeignScan;