Rearrange CustomScan API.

Make it work more like FDW plans do: instead of assuming that there are
expressions in a CustomScan plan node that the core code doesn't know
about, insist that all subexpressions that need planner attention be in
a "custom_exprs" list in the Plan representation.  (Of course, the
custom plugin can break the list apart again at executor initialization.)
This lets us revert the parts of the patch that exposed setrefs.c and
subselect.c processing to the outside world.

Also revert the GetSpecialCustomVar stuff in ruleutils.c; that concept
may work in future, but it's far from fully baked right now.

src/backend/nodes/copyfuncs.c

@@ -603,17 +603,24 @@ _copyForeignScan(const ForeignScan *from)
 static CustomScan *
 _copyCustomScan(const CustomScan *from)
 {
-	CustomScan		   *newnode;
+	CustomScan *newnode = makeNode(CustomScan);
 
-	newnode = from->methods->CopyCustomScan(from);
-	Assert(nodeTag(newnode) == nodeTag(from));
-
-	CopyScanFields((const Scan *) from, (Scan *) newnode);
-	COPY_SCALAR_FIELD(flags);
 	/*
-	 * NOTE: The method field of CustomScan is required to be a pointer
+	 * copy node superclass fields
-	 * to a static table of callback functions. So, we don't copy the
+	 */
-	 * table itself, just reference the original one.
+	CopyScanFields((const Scan *) from, (Scan *) newnode);
+
+	/*
+	 * copy remainder of node
+	 */
+	COPY_SCALAR_FIELD(flags);
+	COPY_NODE_FIELD(custom_exprs);
+	COPY_NODE_FIELD(custom_private);
+
+	/*
+	 * NOTE: The method field of CustomScan is required to be a pointer to a
+	 * static table of callback functions.  So we don't copy the table itself,
+	 * just reference the original one.
 	 */
 	COPY_SCALAR_FIELD(methods);
 

src/backend/nodes/outfuncs.c

@@ -569,9 +569,13 @@ _outCustomScan(StringInfo str, const CustomScan *node)
 	WRITE_NODE_TYPE("CUSTOMSCAN");
 
 	_outScanInfo(str, (const Scan *) node);
+
 	WRITE_UINT_FIELD(flags);
-	appendStringInfo(str, " :methods");
+	WRITE_NODE_FIELD(custom_exprs);
+	WRITE_NODE_FIELD(custom_private);
+	appendStringInfoString(str, " :methods ");
 	_outToken(str, node->methods->CustomName);
+	if (node->methods->TextOutCustomScan)
 		node->methods->TextOutCustomScan(str, node);
 }
 
@@ -1600,10 +1604,14 @@ static void
 _outCustomPath(StringInfo str, const CustomPath *node)
 {
 	WRITE_NODE_TYPE("CUSTOMPATH");
+
 	_outPathInfo(str, (const Path *) node);
+
 	WRITE_UINT_FIELD(flags);
-	appendStringInfo(str, " :methods");
+	WRITE_NODE_FIELD(custom_private);
+	appendStringInfoString(str, " :methods ");
 	_outToken(str, node->methods->CustomName);
+	if (node->methods->TextOutCustomPath)
 		node->methods->TextOutCustomPath(str, node);
 }
 

src/backend/optimizer/plan/createplan.c

@@ -77,7 +77,7 @@ static WorkTableScan *create_worktablescan_plan(PlannerInfo *root, Path *best_pa
 						  List *tlist, List *scan_clauses);
 static ForeignScan *create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
 						List *tlist, List *scan_clauses);
-static Plan *create_customscan_plan(PlannerInfo *root,
+static CustomScan *create_customscan_plan(PlannerInfo *root,
 					   CustomPath *best_path,
 					   List *tlist, List *scan_clauses);
 static NestLoop *create_nestloop_plan(PlannerInfo *root, NestPath *best_path,
@@ -86,6 +86,7 @@ static MergeJoin *create_mergejoin_plan(PlannerInfo *root, MergePath *best_path,
 					  Plan *outer_plan, Plan *inner_plan);
 static HashJoin *create_hashjoin_plan(PlannerInfo *root, HashPath *best_path,
 					 Plan *outer_plan, Plan *inner_plan);
+static Node *replace_nestloop_params(PlannerInfo *root, Node *expr);
 static Node *replace_nestloop_params_mutator(Node *node, PlannerInfo *root);
 static void process_subquery_nestloop_params(PlannerInfo *root,
 								 List *subplan_params);
@@ -413,7 +414,7 @@ create_scan_plan(PlannerInfo *root, Path *best_path)
 			break;
 
 		case T_CustomScan:
-			plan = create_customscan_plan(root,
+			plan = (Plan *) create_customscan_plan(root,
 												   (CustomPath *) best_path,
 												   tlist,
 												   scan_clauses);
@@ -2022,11 +2023,11 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
  *
  * Transform a CustomPath into a Plan.
  */
-static Plan *
+static CustomScan *
 create_customscan_plan(PlannerInfo *root, CustomPath *best_path,
 					   List *tlist, List *scan_clauses)
 {
-	Plan	   *plan;
+	CustomScan *cplan;
 	RelOptInfo *rel = best_path->path.parent;
 
 	/*
@@ -2045,23 +2046,35 @@ create_customscan_plan(PlannerInfo *root, CustomPath *best_path,
 	 * Invoke custom plan provider to create the Plan node represented by the
 	 * CustomPath.
 	 */
-	plan = best_path->methods->PlanCustomPath(root, rel, best_path, tlist,
+	cplan = (CustomScan *) best_path->methods->PlanCustomPath(root,
+															  rel,
+															  best_path,
+															  tlist,
 															  scan_clauses);
-
+	Assert(IsA(cplan, CustomScan));
-	/*
-	 * NOTE: unlike create_foreignscan_plan(), it is the responsibility of the
-	 * custom plan provider to replace outer-relation variables with nestloop
-	 * params, because we cannot know what expression trees may be held in
-	 * private fields.
-	 */
 
 	/*
 	 * Copy cost data from Path to Plan; no need to make custom-plan providers
 	 * do this
 	 */
-	copy_path_costsize(plan, &best_path->path);
+	copy_path_costsize(&cplan->scan.plan, &best_path->path);
 
-	return plan;
+	/*
+	 * Replace any outer-relation variables with nestloop params in the qual
+	 * and custom_exprs expressions.  We do this last so that the custom-plan
+	 * provider doesn't have to be involved.  (Note that parts of custom_exprs
+	 * could have come from join clauses, so doing this beforehand on the
+	 * scan_clauses wouldn't work.)
+	 */
+	if (best_path->path.param_info)
+	{
+		cplan->scan.plan.qual = (List *)
+			replace_nestloop_params(root, (Node *) cplan->scan.plan.qual);
+		cplan->custom_exprs = (List *)
+			replace_nestloop_params(root, (Node *) cplan->custom_exprs);
+	}
+
+	return cplan;
 }
 
 
@@ -2598,7 +2611,7 @@ create_hashjoin_plan(PlannerInfo *root,
  * root->curOuterRels are replaced by Params, and entries are added to
  * root->curOuterParams if not already present.
  */
-Node *
+static Node *
 replace_nestloop_params(PlannerInfo *root, Node *expr)
 {
 	/* No setup needed for tree walk, so away we go */

src/backend/optimizer/plan/setrefs.c

@@ -94,6 +94,7 @@ static Plan *set_subqueryscan_references(PlannerInfo *root,
 							SubqueryScan *plan,
 							int rtoffset);
 static bool trivial_subqueryscan(SubqueryScan *plan);
+static Node *fix_scan_expr(PlannerInfo *root, Node *node, int rtoffset);
 static Node *fix_scan_expr_mutator(Node *node, fix_scan_expr_context *context);
 static bool fix_scan_expr_walker(Node *node, fix_scan_expr_context *context);
 static void set_join_references(PlannerInfo *root, Join *join, int rtoffset);
@@ -580,23 +581,15 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
 
 		case T_CustomScan:
 			{
-				CustomScan *cscan = (CustomScan *) plan;
+				CustomScan *splan = (CustomScan *) plan;
 
-				cscan->scan.scanrelid += rtoffset;
+				splan->scan.scanrelid += rtoffset;
-				cscan->scan.plan.targetlist =
+				splan->scan.plan.targetlist =
-					fix_scan_list(root, cscan->scan.plan.targetlist, rtoffset);
+					fix_scan_list(root, splan->scan.plan.targetlist, rtoffset);
-				cscan->scan.plan.qual =
+				splan->scan.plan.qual =
-					fix_scan_list(root, cscan->scan.plan.qual, rtoffset);
+					fix_scan_list(root, splan->scan.plan.qual, rtoffset);
-
+				splan->custom_exprs =
-				/*
+					fix_scan_list(root, splan->custom_exprs, rtoffset);
-				 * The core implementation applies the routine to fixup varno
-				 * on the target-list and scan qualifier. If custom-scan has
-				 * additional expression nodes on its private fields, it has
-				 * to apply same fixup on them. Otherwise, the custom-plan
-				 * provider can skip this callback.
-				 */
-				if (cscan->methods->SetCustomScanRef)
-					cscan->methods->SetCustomScanRef(root, cscan, rtoffset);
 			}
 			break;
 
@@ -1182,7 +1175,7 @@ fix_param_node(PlannerInfo *root, Param *p)
  * looking up operator opcode info for OpExpr and related nodes,
  * and adding OIDs from regclass Const nodes into root->glob->relationOids.
  */
-Node *
+static Node *
 fix_scan_expr(PlannerInfo *root, Node *node, int rtoffset)
 {
 	fix_scan_expr_context context;

src/backend/optimizer/plan/subselect.c

@@ -2284,24 +2284,9 @@ finalize_plan(PlannerInfo *root, Plan *plan, Bitmapset *valid_params,
 			break;
 
 		case T_CustomScan:
-			{
+			finalize_primnode((Node *) ((CustomScan *) plan)->custom_exprs,
-				CustomScan *custom_scan = (CustomScan *) plan;
+							  &context);
-
+			context.paramids = bms_add_members(context.paramids, scan_params);
-				context.paramids = bms_add_members(context.paramids,
-												   scan_params);
-				/*
-				 * custom-scan provider is responsible to apply
-				 * finalize_primnode() on the expression node of
-				 * its private fields, but no need to apply it
-				 * on the tlist and qual of Plan node because it
-				 * is already done above.
-				 */
-				if (custom_scan->methods->FinalizeCustomScan)
-					custom_scan->methods->FinalizeCustomScan(root,
-															 custom_scan,
-															 finalize_primnode,
-															 (void *)&context);
-			}
 			break;
 
 		case T_ModifyTable:

src/backend/utils/adt/ruleutils.c

@@ -5493,26 +5493,6 @@ get_utility_query_def(Query *query, deparse_context *context)
 	}
 }
 
-/*
- * GetSpecialCustomVar
- *
- * If a custom-scan provider uses a special varnode, this function will be
- * called when deparsing; it should return an Expr node to be reversed-listed
- * in lieu of the special Var.
- */
-static Node *
-GetSpecialCustomVar(CustomScanState *css, Var *varnode, PlanState **child_ps)
-{
-	Assert(IsA(css, CustomScanState));
-	Assert(IS_SPECIAL_VARNO(varnode->varno));
-
-	if (!css->methods->GetSpecialCustomVar)
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("%s does not support special varno reference",
-						css->methods->CustomName)));
-	return (Node *) css->methods->GetSpecialCustomVar(css, varnode, child_ps);
-}
 
 /*
  * Display a Var appropriately.
@@ -5542,8 +5522,6 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
 	int			netlevelsup;
 	deparse_namespace *dpns;
 	deparse_columns *colinfo;
-	PlanState  *child_ps = NULL;
-	Node	   *expr;
 	char	   *refname;
 	char	   *attname;
 
@@ -5568,29 +5546,6 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
 		colinfo = deparse_columns_fetch(var->varno, dpns);
 		attnum = var->varattno;
 	}
-	else if (IS_SPECIAL_VARNO(var->varno) &&
-			 IsA(dpns->planstate, CustomScanState) &&
-			 (expr = GetSpecialCustomVar((CustomScanState *) dpns->planstate,
-										 var, &child_ps)) != NULL)
-	{
-		deparse_namespace	save_dpns;
-
-		if (child_ps)
-			push_child_plan(dpns, child_ps, &save_dpns);
-		/*
-		 * Force parentheses because our caller probably assumed a Var is a
-		 * simple expression.
-		 */
-		if (!IsA(expr, Var))
-			appendStringInfoChar(buf, '(');
-		get_rule_expr((Node *) expr, context, true);
-		if (!IsA(expr, Var))
-			appendStringInfoChar(buf, ')');
-
-		if (child_ps)
-			pop_child_plan(dpns, &save_dpns);
-		return NULL;
-	}
 	else if (var->varno == OUTER_VAR && dpns->outer_tlist)
 	{
 		TargetEntry *tle;
@@ -5805,7 +5760,6 @@ get_name_for_var_field(Var *var, int fieldno,
 	AttrNumber	attnum;
 	int			netlevelsup;
 	deparse_namespace *dpns;
-	PlanState  *child_ps = NULL;
 	TupleDesc	tupleDesc;
 	Node	   *expr;
 
@@ -5880,30 +5834,6 @@ get_name_for_var_field(Var *var, int fieldno,
 		rte = rt_fetch(var->varno, dpns->rtable);
 		attnum = var->varattno;
 	}
-	else if (IS_SPECIAL_VARNO(var->varno) &&
-			 IsA(dpns->planstate, CustomScanState) &&
-			 (expr = GetSpecialCustomVar((CustomScanState *) dpns->planstate,
-										 var, &child_ps)) != NULL)
-	{
-		StringInfo		saved = context->buf;
-		StringInfoData	temp;
-		deparse_namespace save_dpns;
-
-		initStringInfo(&temp);
-		context->buf = &temp;
-
-		if (child_ps)
-			push_child_plan(dpns, child_ps, &save_dpns);
-		if (!IsA(expr, Var))
-			appendStringInfoChar(context->buf, '(');
-		get_rule_expr((Node *) expr, context, true);
-		if (!IsA(expr, Var))
-			appendStringInfoChar(context->buf, ')');
-		if (child_ps)
-			pop_child_plan(dpns, &save_dpns);
-		context->buf = saved;
-		return temp.data;
-	}
 	else if (var->varno == OUTER_VAR && dpns->outer_tlist)
 	{
 		TargetEntry *tle;

src/include/executor/nodeCustom.h

@@ -20,7 +20,6 @@
 extern CustomScanState *ExecInitCustomScan(CustomScan *custom_scan,
 				   EState *estate, int eflags);
 extern TupleTableSlot *ExecCustomScan(CustomScanState *node);
-extern Node *MultiExecCustomScan(CustomScanState *node);
 extern void ExecEndCustomScan(CustomScanState *node);
 
 extern void ExecReScanCustomScan(CustomScanState *node);

src/include/nodes/execnodes.h

@@ -1506,6 +1506,13 @@ typedef struct ForeignScanState
  *	 CustomScanState information
  *
  *		CustomScan nodes are used to execute custom code within executor.
+ *
+ * Core code must avoid assuming that the CustomScanState is only as large as
+ * the structure declared here; providers are allowed to make it the first
+ * element in a larger structure, and typically would need to do so.  The
+ * struct is actually allocated by the CreateCustomScanState method associated
+ * with the plan node.  Any additional fields can be initialized there, or in
+ * the BeginCustomScan method.
  * ----------------
  */
 struct ExplainState;			/* avoid including explain.h here */
@@ -1515,7 +1522,7 @@ typedef struct CustomExecMethods
 {
 	const char *CustomName;
 
-	/* EXECUTOR methods */
+	/* Executor methods: mark/restore are optional, the rest are required */
 	void		(*BeginCustomScan) (struct CustomScanState *node,
 												EState *estate,
 												int eflags);
@@ -1525,13 +1532,10 @@ typedef struct CustomExecMethods
 	void		(*MarkPosCustomScan) (struct CustomScanState *node);
 	void		(*RestrPosCustomScan) (struct CustomScanState *node);
 
-	/* EXPLAIN support */
+	/* Optional: print additional information in EXPLAIN */
 	void		(*ExplainCustomScan) (struct CustomScanState *node,
 												  List *ancestors,
 												  struct ExplainState *es);
-	Node	   *(*GetSpecialCustomVar) (struct CustomScanState *node,
-													Var *varnode,
-													PlanState **child_ps);
 } CustomExecMethods;
 
 typedef struct CustomScanState

src/include/nodes/plannodes.h

@@ -486,32 +486,32 @@ typedef struct ForeignScan
 
 /* ----------------
  *	   CustomScan node
+ *
+ * The comments for ForeignScan's fdw_exprs and fdw_private fields apply
+ * equally to custom_exprs and custom_private.  Note that since Plan trees
+ * can be copied, custom scan providers *must* fit all plan data they need
+ * into those fields; embedding CustomScan in a larger struct will not work.
  * ----------------
  */
-struct PlannerInfo;				/* avoid including relation.h here */
 struct CustomScan;
 
 typedef struct CustomScanMethods
 {
 	const char *CustomName;
 
-	void		(*SetCustomScanRef) (struct PlannerInfo *root,
+	/* Create execution state (CustomScanState) from a CustomScan plan node */
-												 struct CustomScan *cscan,
-												 int rtoffset);
-	void		(*FinalizeCustomScan) (struct PlannerInfo *root,
-												   struct CustomScan *cscan,
-												bool (*finalize_primnode) (),
-												   void *finalize_context);
 	Node	   *(*CreateCustomScanState) (struct CustomScan *cscan);
+	/* Optional: print custom_xxx fields in some special way */
 	void		(*TextOutCustomScan) (StringInfo str,
 											  const struct CustomScan *node);
-	struct CustomScan *(*CopyCustomScan) (const struct CustomScan *from);
 } CustomScanMethods;
 
 typedef struct CustomScan
 {
 	Scan		scan;
 	uint32		flags;			/* mask of CUSTOMPATH_* flags, see relation.h */
+	List	   *custom_exprs;	/* expressions that custom code may evaluate */
+	List	   *custom_private; /* private data for custom code */
 	const CustomScanMethods *methods;
 } CustomScan;
 

src/include/nodes/relation.h

@@ -885,17 +885,22 @@ typedef struct ForeignPath
 } ForeignPath;
 
 /*
- * CustomPath represents a scan by some out-of-core extension.
+ * CustomPath represents a table scan done by some out-of-core extension.
  *
- * We provide a set of hooks here - which the provider must take care to
+ * We provide a set of hooks here - which the provider must take care to set
- * set up correctly - to allow extensions to supply their own methods of
+ * up correctly - to allow extensions to supply their own methods of scanning
- * scanning a relation.  For example, a provider might provide GPU
+ * a relation.  For example, a provider might provide GPU acceleration, a
- * acceleration, a cache-based scan, or some other kind of logic we haven't
+ * cache-based scan, or some other kind of logic we haven't dreamed up yet.
- * dreamed up yet.
  *
- * Core code should avoid assuming that the CustomPath is only as large as
+ * CustomPaths can be injected into the planning process for a relation by
- * the structure declared here; providers are expected to make it the first
+ * set_rel_pathlist_hook functions.
- * element in a larger structure.
+ *
+ * Core code must avoid assuming that the CustomPath is only as large as
+ * the structure declared here; providers are allowed to make it the first
+ * element in a larger structure.  (Since the planner never copies Paths,
+ * this doesn't add any complication.)  However, for consistency with the
+ * FDW case, we provide a "custom_private" field in CustomPath; providers
+ * may prefer to use that rather than define another struct type.
  */
 struct CustomPath;
 
@@ -906,11 +911,13 @@ typedef struct CustomPathMethods
 {
 	const char *CustomName;
 
+	/* Convert Path to a Plan */
 	struct Plan *(*PlanCustomPath) (PlannerInfo *root,
 												RelOptInfo *rel,
 												struct CustomPath *best_path,
 												List *tlist,
 												List *clauses);
+	/* Optional: print additional fields besides "private" */
 	void		(*TextOutCustomPath) (StringInfo str,
 											  const struct CustomPath *node);
 } CustomPathMethods;
@@ -919,6 +926,7 @@ typedef struct CustomPath
 {
 	Path		path;
 	uint32		flags;			/* mask of CUSTOMPATH_* flags, see above */
+	List	   *custom_private;
 	const CustomPathMethods *methods;
 } CustomPath;
 

src/include/optimizer/planmain.h

@@ -86,7 +86,6 @@ extern ModifyTable *make_modifytable(PlannerInfo *root,
 				 List *withCheckOptionLists, List *returningLists,
 				 List *rowMarks, int epqParam);
 extern bool is_projection_capable_plan(Plan *plan);
-extern Node *replace_nestloop_params(PlannerInfo *root, Node *expr);
 
 /*
  * prototypes for plan/initsplan.c
@@ -130,7 +129,6 @@ extern bool query_is_distinct_for(Query *query, List *colnos, List *opids);
  * prototypes for plan/setrefs.c
  */
 extern Plan *set_plan_references(PlannerInfo *root, Plan *plan);
-extern Node *fix_scan_expr(PlannerInfo *root, Node *node, int rtoffset);
 extern void fix_opfuncids(Node *node);
 extern void set_opfuncid(OpExpr *opexpr);
 extern void set_sa_opfuncid(ScalarArrayOpExpr *opexpr);