Initial code review for CustomScan patch.

Get rid of the pernicious entanglement between planner and executor headers
introduced by commit 0b03e5951bf0a1a8868db13f02049cf686a82165.

Also, rearrange the CustomFoo struct/typedef definitions so that all the
typedef names are seen as used by the compiler.  Without this pgindent
will mess things up a bit, which is not so important perhaps, but it also
removes a bizarre discrepancy between the declaration arrangement used for
CustomExecMethods and that used for CustomScanMethods and
CustomPathMethods.

Clean up the commentary around ExecSupportsMarkRestore to reflect the
rather large change in its API.

Const-ify register_custom_path_provider's argument.  This necessitates
casting away const in the function, but that seems better than forcing
callers of the function to do so (or else not const-ify their method
pointer structs, which was sort of the whole point).

De-export fix_expr_common.  I don't like the exporting of fix_scan_expr
or replace_nestloop_params either, but this one surely has got little
excuse.

7 files changed, 76 insertions, 75 deletions

diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index f1b65b4d050..ed3ae39b66d 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -16,7 +16,6 @@
 
 #include "executor/execdesc.h"
 #include "nodes/parsenodes.h"
-#include "nodes/relation.h"
 #include "utils/lockwaitpolicy.h"
 
 
@@ -101,10 +100,12 @@ extern PGDLLIMPORT ExecutorCheckPerms_hook_type ExecutorCheckPerms_hook;
 /*
  * prototypes from functions in execAmi.c
  */
+struct Path;					/* avoid including relation.h here */
+
 extern void ExecReScan(PlanState *node);
 extern void ExecMarkPos(PlanState *node);
 extern void ExecRestrPos(PlanState *node);
-extern bool ExecSupportsMarkRestore(Path *pathnode);
+extern bool ExecSupportsMarkRestore(struct Path *pathnode);
 extern bool ExecSupportsBackwardScan(Plan *node);
 extern bool ExecMaterializesOutput(NodeTag plantype);
 
diff --git a/src/include/executor/nodeCustom.h b/src/include/executor/nodeCustom.h
index 1736d48bfaf..e6f125544bc 100644
--- a/src/include/executor/nodeCustom.h
+++ b/src/include/executor/nodeCustom.h
@@ -11,14 +11,14 @@
  */
 #ifndef NODECUSTOM_H
 #define NODECUSTOM_H
-#include "nodes/plannodes.h"
+
 #include "nodes/execnodes.h"
 
 /*
  * General executor code
  */
 extern CustomScanState *ExecInitCustomScan(CustomScan *custom_scan,
-										   EState *estate, int eflags);
+				   EState *estate, int eflags);
 extern TupleTableSlot *ExecCustomScan(CustomScanState *node);
 extern Node *MultiExecCustomScan(CustomScanState *node);
 extern void ExecEndCustomScan(CustomScanState *node);
@@ -27,4 +27,4 @@ extern void ExecReScanCustomScan(CustomScanState *node);
 extern void ExecCustomMarkPos(CustomScanState *node);
 extern void ExecCustomRestrPos(CustomScanState *node);
 
-#endif	/* NODECUSTOM_H */
+#endif   /* NODECUSTOM_H */
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 8c8c01f1cd2..40fb8243ab4 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -19,7 +19,6 @@
 #include "executor/instrument.h"
 #include "nodes/params.h"
 #include "nodes/plannodes.h"
-#include "nodes/relation.h"
 #include "utils/reltrigger.h"
 #include "utils/sortsupport.h"
 #include "utils/tuplestore.h"
@@ -1512,39 +1511,39 @@ typedef struct ForeignScanState
  *		CustomScan nodes are used to execute custom code within executor.
  * ----------------
  */
-struct CustomExecMethods;
-struct ExplainState;	/* to avoid to include explain.h here */
-
-typedef struct CustomScanState
-{
-	ScanState	ss;
-	uint32		flags;	/* mask of CUSTOMPATH_* flags defined in relation.h*/
-	const struct CustomExecMethods *methods;
-} CustomScanState;
+struct ExplainState;			/* avoid including explain.h here */
+struct CustomScanState;
 
 typedef struct CustomExecMethods
 {
-	const char     *CustomName;
+	const char *CustomName;
 
 	/* EXECUTOR methods */
-	void    (*BeginCustomScan)(CustomScanState *node,
-							   EState *estate,
-							   int eflags);
-	TupleTableSlot *(*ExecCustomScan)(CustomScanState *node);
-	void	(*EndCustomScan)(CustomScanState *node);
-	void	(*ReScanCustomScan)(CustomScanState *node);
-	void	(*MarkPosCustomScan)(CustomScanState *node);
-	void	(*RestrPosCustomScan)(CustomScanState *node);
+	void		(*BeginCustomScan) (struct CustomScanState *node,
+												EState *estate,
+												int eflags);
+	TupleTableSlot *(*ExecCustomScan) (struct CustomScanState *node);
+	void		(*EndCustomScan) (struct CustomScanState *node);
+	void		(*ReScanCustomScan) (struct CustomScanState *node);
+	void		(*MarkPosCustomScan) (struct CustomScanState *node);
+	void		(*RestrPosCustomScan) (struct CustomScanState *node);
 
 	/* EXPLAIN support */
-	void    (*ExplainCustomScan)(CustomScanState *node,
-								 List *ancestors,
-								 struct ExplainState *es);
-	Node   *(*GetSpecialCustomVar)(CustomScanState *node,
-								   Var *varnode,
-								   PlanState **child_ps);
+	void		(*ExplainCustomScan) (struct CustomScanState *node,
+												  List *ancestors,
+												  struct ExplainState *es);
+	Node	   *(*GetSpecialCustomVar) (struct CustomScanState *node,
+													Var *varnode,
+													PlanState **child_ps);
 } CustomExecMethods;
 
+typedef struct CustomScanState
+{
+	ScanState	ss;
+	uint32		flags;			/* mask of CUSTOMPATH_* flags, see relation.h */
+	const CustomExecMethods *methods;
+} CustomScanState;
+
 /* ----------------------------------------------------------------
  *				 Join State Information
  * ----------------------------------------------------------------
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 9dbb91cb90d..dd300b1a191 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -18,7 +18,6 @@
 #include "lib/stringinfo.h"
 #include "nodes/bitmapset.h"
 #include "nodes/primnodes.h"
-#include "nodes/relation.h"
 #include "utils/lockwaitpolicy.h"
 
 
@@ -486,33 +485,36 @@ typedef struct ForeignScan
 } ForeignScan;
 
 /* ----------------
- *     CustomScan node
+ *	   CustomScan node
  * ----------------
  */
-struct CustomScanMethods;
-
-typedef struct CustomScan
-{
-	Scan		scan;
-	uint32		flags;	/* mask of CUSTOMPATH_* flags defined in relation.h */
-	struct CustomScanMethods *methods;
-} CustomScan;
+struct PlannerInfo;				/* avoid including relation.h here */
+struct CustomScan;
 
 typedef struct CustomScanMethods
 {
 	const char *CustomName;
-	void	   (*SetCustomScanRef)(struct PlannerInfo *root,
-								   CustomScan *cscan,
-								   int rtoffset);
-	void	   (*FinalizeCustomScan)(struct PlannerInfo *root,
-									 CustomScan *cscan,
-									 bool (*finalize_primnode)(),
-									 void *finalize_context);
-	Node	  *(*CreateCustomScanState)(CustomScan *cscan);
-	void	   (*TextOutCustomScan)(StringInfo str, const CustomScan *node);
-	CustomScan *(*CopyCustomScan)(const CustomScan *from);
+
+	void		(*SetCustomScanRef) (struct PlannerInfo *root,
+												 struct CustomScan *cscan,
+												 int rtoffset);
+	void		(*FinalizeCustomScan) (struct PlannerInfo *root,
+												   struct CustomScan *cscan,
+												bool (*finalize_primnode) (),
+												   void *finalize_context);
+	Node	   *(*CreateCustomScanState) (struct CustomScan *cscan);
+	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 */
+	const CustomScanMethods *methods;
+} CustomScan;
+
 /*
  * ==========
  * Join nodes
@@ -864,7 +866,7 @@ typedef struct PlanRowMark
 	Index		prti;			/* range table index of parent relation */
 	Index		rowmarkId;		/* unique identifier for resjunk columns */
 	RowMarkType markType;		/* see enum above */
-	LockWaitPolicy waitPolicy;  /* NOWAIT and SKIP LOCKED options */
+	LockWaitPolicy waitPolicy;	/* NOWAIT and SKIP LOCKED options */
 	bool		isParent;		/* true if this is a "dummy" parent entry */
 } PlanRowMark;
 
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index 05cfbcd2aa1..7953bf7ed6c 100644
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -897,34 +897,34 @@ typedef struct ForeignPath
  * the structure declared here; providers are expected to make it the first
  * element in a larger structure.
  */
-
-struct CustomPathMethods;
-struct Plan;		/* not to include plannodes.h here */
+struct CustomPath;
 
 #define CUSTOMPATH_SUPPORT_BACKWARD_SCAN	0x0001
 #define CUSTOMPATH_SUPPORT_MARK_RESTORE		0x0002
 
-typedef struct CustomPath
-{
-	Path        path;
-	uint32		flags;
-	const struct CustomPathMethods *methods;
-} CustomPath;
-
 typedef struct CustomPathMethods
 {
 	const char *CustomName;
-	void	(*CreateCustomScanPath)(PlannerInfo *root,
-									RelOptInfo *baserel,
-									RangeTblEntry *rte);
-	struct Plan	*(*PlanCustomPath)(PlannerInfo *root,
-								   RelOptInfo *rel,
-								   CustomPath *best_path,
-								   List *tlist,
-								   List *clauses);
-	void    (*TextOutCustomPath)(StringInfo str, const CustomPath *node);
+
+	void		(*CreateCustomScanPath) (PlannerInfo *root,
+													 RelOptInfo *baserel,
+													 RangeTblEntry *rte);
+	struct Plan *(*PlanCustomPath) (PlannerInfo *root,
+												RelOptInfo *rel,
+												struct CustomPath *best_path,
+												List *tlist,
+												List *clauses);
+	void		(*TextOutCustomPath) (StringInfo str,
+											  const struct CustomPath *node);
 } CustomPathMethods;
 
+typedef struct CustomPath
+{
+	Path		path;
+	uint32		flags;			/* mask of CUSTOMPATH_* flags, see above */
+	const CustomPathMethods *methods;
+} CustomPath;
+
 /*
  * AppendPath represents an Append plan, ie, successive execution of
  * several member plans.
diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h
index 2b67ae6187b..0f2882986c0 100644
--- a/src/include/optimizer/pathnode.h
+++ b/src/include/optimizer/pathnode.h
@@ -131,11 +131,11 @@ extern Path *reparameterize_path(PlannerInfo *root, Path *path,
 /*
  * Interface definition of custom-scan providers
  */
-extern void register_custom_path_provider(CustomPathMethods *cpp_methods);
+extern void register_custom_path_provider(const CustomPathMethods *cpp_methods);
 
 extern void create_customscan_paths(PlannerInfo *root,
-									RelOptInfo *baserel,
-									RangeTblEntry *rte);
+						RelOptInfo *baserel,
+						RangeTblEntry *rte);
 
 /*
  * prototypes for relnode.c
diff --git a/src/include/optimizer/planmain.h b/src/include/optimizer/planmain.h
index c97c5777a07..d6dae8e738e 100644
--- a/src/include/optimizer/planmain.h
+++ b/src/include/optimizer/planmain.h
@@ -130,9 +130,8 @@ 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 void fix_opfuncids(Node *node);
 extern Node *fix_scan_expr(PlannerInfo *root, Node *node, int rtoffset);
-extern void fix_expr_common(PlannerInfo *root, Node *node);
+extern void fix_opfuncids(Node *node);
 extern void set_opfuncid(OpExpr *opexpr);
 extern void set_sa_opfuncid(ScalarArrayOpExpr *opexpr);
 extern void record_plan_function_dependency(PlannerInfo *root, Oid funcid);