Clean up out/readfuncs.c

- Don't write Path->parent, to avoid infinite recursion, and revert the
  GPDB changes in outPlannerInfo that were previously used to avoid it.
  There's no need to differ from upstream in this area, so let's not.

- Add missing window_pathkeys handling from outPlannerInfo. Was missed in
  a merge, I guess. No reason to differ from upstream.

- Remove out/readfast support for Paths and other structs that are only
  used within the planner, and not needed in segments.

- Don't serialize Flow->hashExprs. It's also not needed in segments, and
  it might contain PlaceHolderVars because it's not processed by
  set_plan_references()

The infinite recursion issue was spotted by Melanie and Deep.
Reviewed-by: NGeorgios Kokolatos <gkokolatos@pivotal.io>
Reviewed-by: NSoumyadeep Chakraborty <sochakraborty@pivotal.io>

src/backend/nodes/outfast.c

@@ -655,42 +655,6 @@ _outJoinExpr(StringInfo str, JoinExpr *node)
 	WRITE_INT_FIELD(rtindex);
 }
 
-/*****************************************************************************
- *
- *	Stuff from relation.h.
- *
- *****************************************************************************/
-
-static void
-_outIndexOptInfo(StringInfo str, IndexOptInfo *node)
-{
-	WRITE_NODE_TYPE("INDEXOPTINFO");
-
-	/* NB: this isn't a complete set of fields */
-	WRITE_OID_FIELD(indexoid);
-	/* Do NOT print rel field, else infinite recursion */
-	WRITE_UINT_FIELD(pages);
-	WRITE_FLOAT_FIELD(tuples, "%.0f");
-	WRITE_INT_FIELD(ncolumns);
-
-	WRITE_OID_ARRAY(opfamily, node->ncolumns);
-	WRITE_INT_ARRAY(indexkeys, node->ncolumns, int);
-	WRITE_OID_ARRAY(sortopfamily, node->ncolumns);
-
-	WRITE_BOOL_FIELD(reverse_sort);
-	WRITE_BOOL_FIELD(nulls_first);
-
-    WRITE_OID_FIELD(relam);
-	WRITE_OID_FIELD(amcostestimate);
-	WRITE_NODE_FIELD(indexprs);
-	WRITE_NODE_FIELD(indpred);
-	WRITE_BOOL_FIELD(predOK);
-	WRITE_BOOL_FIELD(unique);
-	WRITE_BOOL_FIELD(hypothetical);
-
-	WRITE_BOOL_FIELD(amoptionalkey);
-}
-
 /*****************************************************************************
  *
  *	Stuff from parsenodes.h.
@@ -1701,95 +1665,10 @@ _outNode(StringInfo str, void *obj)
 			case T_OnConflictExpr:
 				_outOnConflictExpr(str, obj);
 				break;
-			case T_Path:
-				_outPath(str, obj);
-				break;
-			case T_IndexPath:
-				_outIndexPath(str, obj);
-				break;
-			case T_BitmapHeapPath:
-				_outBitmapHeapPath(str, obj);
-				break;
-			case T_BitmapAndPath:
-				_outBitmapAndPath(str, obj);
-				break;
-			case T_BitmapOrPath:
-				_outBitmapOrPath(str, obj);
-				break;
-			case T_TidPath:
-				_outTidPath(str, obj);
-				break;
-			case T_ForeignPath:
-				_outForeignPath(str, obj);
-				break;
-			case T_AppendPath:
-				_outAppendPath(str, obj);
-				break;
-			case T_MergeAppendPath:
-				_outMergeAppendPath(str, obj);
-				break;
-			case T_AppendOnlyPath:
-				_outAppendOnlyPath(str, obj);
-				break;
-			case T_AOCSPath:
-				_outAOCSPath(str, obj);
-				break;
-			case T_ResultPath:
-				_outResultPath(str, obj);
-				break;
-			case T_MaterialPath:
-				_outMaterialPath(str, obj);
-				break;
-			case T_UniquePath:
-				_outUniquePath(str, obj);
-				break;
-			case T_NestPath:
-				_outNestPath(str, obj);
-				break;
-			case T_MergePath:
-				_outMergePath(str, obj);
-				break;
-			case T_HashPath:
-				_outHashPath(str, obj);
-				break;
-            case T_CdbMotionPath:
-                _outCdbMotionPath(str, obj);
-                break;
-			case T_PlannerInfo:
-				_outPlannerInfo(str, obj);
-				break;
-			case T_PlannerParamItem:
-				_outPlannerParamItem(str, obj);
-				break;
-			case T_RelOptInfo:
-				_outRelOptInfo(str, obj);
-				break;
-			case T_IndexOptInfo:
-				_outIndexOptInfo(str, obj);
-				break;
-			case T_PathKey:
-				_outPathKey(str, obj);
-				break;
-			case T_ParamPathInfo:
-				_outParamPathInfo(str, obj);
-				break;
-			case T_RestrictInfo:
-				_outRestrictInfo(str, obj);
-				break;
-			case T_SpecialJoinInfo:
-				_outSpecialJoinInfo(str, obj);
-				break;
-			case T_LateralJoinInfo:
-				_outLateralJoinInfo(str, obj);
-				break;
-			case T_AppendRelInfo:
-				_outAppendRelInfo(str, obj);
-				break;
+
 			case T_CreateExtensionStmt:
 				_outCreateExtensionStmt(str, obj);
 				break;
-
-
 			case T_GrantStmt:
 				_outGrantStmt(str, obj);
 				break;
@@ -2271,15 +2150,6 @@ _outNode(StringInfo str, void *obj)
 			case T_AlterTSDictionaryStmt:
 				_outAlterTSDictionaryStmt(str, obj);
 				break;
-			case T_PlaceHolderVar:
-				_outPlaceHolderVar(str, obj);
-				break;
-			case T_PlaceHolderInfo:
-				_outPlaceHolderInfo(str, obj);
-				break;
-			case T_MinMaxAggInfo:
-				_outMinMaxAggInfo(str, obj);
-				break;
 
 			case T_CookedConstraint:
 				_outCookedConstraint(str, obj);

src/backend/nodes/outfuncs.c

@@ -2000,8 +2000,20 @@ _outFlow(StringInfo str, const Flow *node)
 	WRITE_INT_FIELD(segindex);
 	WRITE_INT_FIELD(numsegments);
 
+	/*
+	 * Don't send hashExprs to segments. It's not needed in segments.
+	 * Also, the hash expressions are not processed by set_plan_references(),
+	 * and might therefore still contain PlaceHolderVars, and we're missing
+	 * out/readfast support for them, too.
+	 *
+	 * FIXME: It's a bit ugly that we need the other fields from 'flow'.
+	 * We should clean that up, copying any information that's needed after
+	 * planning elsewhere.
+	 */
+#ifndef COMPILING_BINARY_FUNCS
 	WRITE_NODE_FIELD(hashExprs);
 	WRITE_NODE_FIELD(hashOpfamilies);
+#endif /* COMPILING_BINARY_FUNCS */
 }
 
 /*****************************************************************************
@@ -2013,6 +2025,7 @@ _outFlow(StringInfo str, const Flow *node)
 /*
  * _outCdbPathLocus
  */
+#ifndef COMPILING_BINARY_FUNCS
 static void
 _outCdbPathLocus(StringInfo str, const CdbPathLocus *node)
 {
@@ -2020,6 +2033,7 @@ _outCdbPathLocus(StringInfo str, const CdbPathLocus *node)
 	WRITE_NODE_FIELD(distkey);
 	WRITE_INT_FIELD(numsegments);
 }                               /* _outCdbPathLocus */
+#endif /* COMPILING_BINARY_FUNCS */
 
 static void
 _outOnConflictExpr(StringInfo str, const OnConflictExpr *node)
@@ -2042,6 +2056,12 @@ _outOnConflictExpr(StringInfo str, const OnConflictExpr *node)
  *
  *****************************************************************************/
 
+/*
+ * None of this stuff is needed after planning, and doesn't need to be
+ * dispatched to QEs.
+ */
+#ifndef COMPILING_BINARY_FUNCS
+
 /*
  * print the basic stuff of all nodes that inherit from Path
  *
@@ -2068,7 +2088,6 @@ _outPathInfo(StringInfo str, const Path *node)
 	WRITE_FLOAT_FIELD(rows, "%.0f");
 	WRITE_FLOAT_FIELD(startup_cost, "%.2f");
 	WRITE_FLOAT_FIELD(total_cost, "%.2f");
-    WRITE_NODE_FIELD(parent);
     _outCdbPathLocus(str, &node->locus);
 	WRITE_NODE_FIELD(pathkeys);
 }
@@ -2166,7 +2185,6 @@ _outForeignPath(StringInfo str, const ForeignPath *node)
 	WRITE_NODE_FIELD(fdw_private);
 }
 
-#ifndef COMPILING_BINARY_FUNCS
 static void
 _outCustomPath(StringInfo str, const CustomPath *node)
 {
@@ -2181,7 +2199,6 @@ _outCustomPath(StringInfo str, const CustomPath *node)
 	if (node->methods->TextOutCustomPath)
 		node->methods->TextOutCustomPath(str, node);
 }
-#endif /* COMPILING_BINARY_FUNCS */
 
 static void
 _outAppendPath(StringInfo str, const AppendPath *node)
@@ -2290,7 +2307,6 @@ _outHashPath(StringInfo str, const HashPath *node)
 	WRITE_INT_FIELD(num_batches);
 }
 
-#ifndef COMPILING_BINARY_FUNCS
 static void
 _outProjectionPath(StringInfo str, const ProjectionPath *node)
 {
@@ -2301,7 +2317,6 @@ _outProjectionPath(StringInfo str, const ProjectionPath *node)
 	WRITE_NODE_FIELD(subpath);
 	WRITE_BOOL_FIELD(dummypp);
 }
-#endif /* COMPILING_BINARY_FUNCS */
 
 static void
 _outCdbMotionPath(StringInfo str, const CdbMotionPath *node)
@@ -2313,7 +2328,6 @@ _outCdbMotionPath(StringInfo str, const CdbMotionPath *node)
     WRITE_NODE_FIELD(subpath);
 }
 
-#ifndef COMPILING_BINARY_FUNCS
 static void
 _outPlannerGlobal(StringInfo str, const PlannerGlobal *node)
 {
@@ -2338,7 +2352,6 @@ _outPlannerGlobal(StringInfo str, const PlannerGlobal *node)
 	WRITE_INT_FIELD(share.nextPlanId);
 	WRITE_BOOL_FIELD(hasRowSecurity);
 }
-#endif /* COMPILING_BINARY_FUNCS */
 
 static void
 _outPlannerInfo(StringInfo str, const PlannerInfo *node)
@@ -2369,6 +2382,7 @@ _outPlannerInfo(StringInfo str, const PlannerInfo *node)
 	WRITE_NODE_FIELD(placeholder_list);
 	WRITE_NODE_FIELD(query_pathkeys);
 	WRITE_NODE_FIELD(group_pathkeys);
+	WRITE_NODE_FIELD(window_pathkeys);
 	WRITE_NODE_FIELD(distinct_pathkeys);
 	WRITE_NODE_FIELD(sort_pathkeys);
 	WRITE_NODE_FIELD(minmax_aggs);
@@ -2400,17 +2414,19 @@ _outRelOptInfo(StringInfo str, const RelOptInfo *node)
 	WRITE_BOOL_FIELD(consider_startup);
 	WRITE_BOOL_FIELD(consider_param_startup);
 	WRITE_NODE_FIELD(reltargetlist);
-	/* Skip writing Path ptrs to avoid endless recursion */
-	/* WRITE_NODE_FIELD(pathlist);              */
-	/* WRITE_NODE_FIELD(cheapest_startup_path); */
-	/* WRITE_NODE_FIELD(cheapest_total_path);   */
+	WRITE_NODE_FIELD(pathlist);
+	WRITE_NODE_FIELD(ppilist);
+	WRITE_NODE_FIELD(cheapest_startup_path);
+	WRITE_NODE_FIELD(cheapest_total_path);
+	WRITE_NODE_FIELD(cheapest_unique_path);
+	WRITE_NODE_FIELD(cheapest_parameterized_paths);
+	WRITE_BITMAPSET_FIELD(lateral_relids);
 	WRITE_UINT_FIELD(relid);
 	WRITE_OID_FIELD(reltablespace);
 	WRITE_ENUM_FIELD(rtekind, RTEKind);
 	WRITE_INT_FIELD(min_attr);
 	WRITE_INT_FIELD(max_attr);
 	WRITE_NODE_FIELD(lateral_vars);
-	WRITE_BITMAPSET_FIELD(lateral_relids);
 	WRITE_BITMAPSET_FIELD(lateral_referencers);
 	WRITE_NODE_FIELD(indexlist);
 	WRITE_UINT_FIELD(pages);
@@ -2426,7 +2442,6 @@ _outRelOptInfo(StringInfo str, const RelOptInfo *node)
 	WRITE_BOOL_FIELD(has_eclass_joins);
 }
 
-#ifndef COMPILING_BINARY_FUNCS
 static void
 _outIndexOptInfo(StringInfo str, const IndexOptInfo *node)
 {
@@ -2441,7 +2456,6 @@ _outIndexOptInfo(StringInfo str, const IndexOptInfo *node)
 	WRITE_INT_FIELD(ncolumns);
 	/* array fields aren't really worth the trouble to print */
 	WRITE_OID_FIELD(relam);
-	WRITE_OID_FIELD(amcostestimate);
 	/* indexprs is redundant since we print indextlist */
 	WRITE_NODE_FIELD(indpred);
 	WRITE_NODE_FIELD(indextlist);
@@ -2451,9 +2465,7 @@ _outIndexOptInfo(StringInfo str, const IndexOptInfo *node)
 	WRITE_BOOL_FIELD(hypothetical);
 	/* we don't bother with fields copied from the pg_am entry */
 }
-#endif /* COMPILING_BINARY_FUNCS */
 
-#ifndef COMPILING_BINARY_FUNCS
 static void
 _outEquivalenceClass(StringInfo str, const EquivalenceClass *node)
 {
@@ -2478,9 +2490,7 @@ _outEquivalenceClass(StringInfo str, const EquivalenceClass *node)
 	WRITE_BOOL_FIELD(ec_broken);
 	WRITE_UINT_FIELD(ec_sortref);
 }
-#endif /* COMPILING_BINARY_FUNCS */
 
-#ifndef COMPILING_BINARY_FUNCS
 static void
 _outEquivalenceMember(StringInfo str, const EquivalenceMember *node)
 {
@@ -2493,7 +2503,6 @@ _outEquivalenceMember(StringInfo str, const EquivalenceMember *node)
 	WRITE_BOOL_FIELD(em_is_child);
 	WRITE_OID_FIELD(em_datatype);
 }
-#endif /* COMPILING_BINARY_FUNCS */
 
 static void
 _outPathKey(StringInfo str, const PathKey *node)
@@ -2506,7 +2515,6 @@ _outPathKey(StringInfo str, const PathKey *node)
 	WRITE_BOOL_FIELD(pk_nulls_first);
 }
 
-#ifndef COMPILING_BINARY_FUNCS
 static void
 _outDistributionKey(StringInfo str, const DistributionKey *node)
 {
@@ -2515,7 +2523,6 @@ _outDistributionKey(StringInfo str, const DistributionKey *node)
 	WRITE_NODE_FIELD(dk_eclasses);
 	WRITE_OID_FIELD(dk_opfamily);
 }
-#endif /* COMPILING_BINARY_FUNCS */
 
 static void
 _outParamPathInfo(StringInfo str, const ParamPathInfo *node)
@@ -2645,6 +2652,8 @@ _outPlannerParamItem(StringInfo str, const PlannerParamItem *node)
 	WRITE_INT_FIELD(paramId);
 }
 
+#endif /* COMPILING_BINARY_FUNCS */
+
 /*****************************************************************************
  *
  *	Stuff from parsenodes.h.

src/backend/nodes/readfast.c

@@ -1430,16 +1430,6 @@ _readGrantRoleStmt(void)
 	READ_DONE();
 }
 
-static PlannerParamItem *
-_readPlannerParamItem(void)
-{
-	READ_LOCALS(PlannerParamItem);
-	READ_NODE_FIELD(item);
-	READ_INT_FIELD(paramId);
-
-	READ_DONE();
-}
-
 /*
  * _readPlannedStmt
  */
@@ -2304,8 +2294,7 @@ _readFlow(void)
 	READ_INT_FIELD(segindex);
 	READ_INT_FIELD(numsegments);
 
-	READ_NODE_FIELD(hashExprs);
-	READ_NODE_FIELD(hashOpfamilies);
+	/* hashExprs and hashOpfamilies are omitted */
 
 	READ_DONE();
 }
@@ -2783,34 +2772,6 @@ _readAlterTSDictionaryStmt(void)
 	READ_DONE();
 }
 
-static PlaceHolderVar *
-_readPlaceHolderVar(void)
-{
-	READ_LOCALS(PlaceHolderVar);
-
-	READ_NODE_FIELD(phexpr);
-	READ_BITMAPSET_FIELD(phrels);
-	READ_INT_FIELD(phid);
-	READ_INT_FIELD(phlevelsup);
-
-	READ_DONE();
-}
-
-static PlaceHolderInfo *
-_readPlaceHolderInfo(void)
-{
-	READ_LOCALS(PlaceHolderInfo);
-
-	READ_INT_FIELD(phid);
-	READ_NODE_FIELD(ph_var);
-	READ_BITMAPSET_FIELD(ph_eval_at);
-	READ_BITMAPSET_FIELD(ph_lateral);
-	READ_BITMAPSET_FIELD(ph_needed);
-	READ_INT_FIELD(ph_width);
-
-	READ_DONE();
-}
-
 static CookedConstraint *
 _readCookedConstraint(void)
 {
@@ -3165,10 +3126,7 @@ readNodeBinary(void)
 				return_value = _readOidAssignment();
 				break;
 			case T_Plan:
-					return_value = _readPlan();
-					break;
-			case T_PlannerParamItem:
-				return_value = _readPlannerParamItem();
+				return_value = _readPlan();
 				break;
 			case T_Result:
 				return_value = _readResult();
@@ -3934,12 +3892,6 @@ readNodeBinary(void)
 			case T_AlterTSDictionaryStmt:
 				return_value = _readAlterTSDictionaryStmt();
 				break;
-			case T_PlaceHolderVar:
-				return_value = _readPlaceHolderVar();
-				break;
-			case T_PlaceHolderInfo:
-				return_value = _readPlaceHolderInfo();
-				break;
 
 			case T_CookedConstraint:
 				return_value = _readCookedConstraint();