fix_issues_from_AB_refactoring_changes.patch

text/x-patch

Filename: fix_issues_from_AB_refactoring_changes.patch
Type: text/x-patch
Part: 0
Message: Re: [HACKERS] Partition-wise aggregation/grouping

Patch

Format: unified
File+
contrib/postgres_fdw/postgres_fdw.c 10 10
src/backend/optimizer/plan/planner.c 1 39
src/include/foreign/fdwapi.h 1 1
src/include/nodes/relation.h 30 8
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 87ea18c..ed16f7d 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -353,7 +353,7 @@ static void postgresGetForeignUpperPaths(PlannerInfo *root,
 							 UpperRelationKind stage,
 							 RelOptInfo *input_rel,
 							 RelOptInfo *output_rel,
-							 OtherUpperPathExtraData *extra);
+							 GroupPathExtraData *group_extra);
 
 /*
  * Helper functions
@@ -429,7 +429,7 @@ static void add_paths_with_pathkeys_for_rel(PlannerInfo *root, RelOptInfo *rel,
 static void add_foreign_grouping_paths(PlannerInfo *root,
 						   RelOptInfo *input_rel,
 						   RelOptInfo *grouped_rel,
-						   OtherUpperPathExtraData *extra);
+						   GroupPathExtraData *group_extra);
 static void apply_server_options(PgFdwRelationInfo *fpinfo);
 static void apply_table_options(PgFdwRelationInfo *fpinfo);
 static void merge_fdw_options(PgFdwRelationInfo *fpinfo,
@@ -5235,7 +5235,7 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel)
 static void
 postgresGetForeignUpperPaths(PlannerInfo *root, UpperRelationKind stage,
 							 RelOptInfo *input_rel, RelOptInfo *output_rel,
-							 OtherUpperPathExtraData *extra)
+							 GroupPathExtraData *group_extra)
 {
 	PgFdwRelationInfo *fpinfo;
 
@@ -5255,7 +5255,7 @@ postgresGetForeignUpperPaths(PlannerInfo *root, UpperRelationKind stage,
 	fpinfo->pushdown_safe = false;
 	output_rel->fdw_private = fpinfo;
 
-	add_foreign_grouping_paths(root, input_rel, output_rel, extra);
+	add_foreign_grouping_paths(root, input_rel, output_rel, group_extra);
 }
 
 /*
@@ -5268,7 +5268,7 @@ postgresGetForeignUpperPaths(PlannerInfo *root, UpperRelationKind stage,
 static void
 add_foreign_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
 						   RelOptInfo *grouped_rel,
-						   OtherUpperPathExtraData *extra)
+						   GroupPathExtraData *group_extra)
 {
 	Query	   *parse = root->parse;
 	PgFdwRelationInfo *ifpinfo = input_rel->fdw_private;
@@ -5288,16 +5288,16 @@ add_foreign_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
 	 * Store passed-in target and havingQual in fpinfo. If its a foreign
 	 * partition, then path target and HAVING quals fetched from the root are
 	 * not correct as Vars within it won't match with this child relation.
-	 * However, server passed them through extra and thus fetch from it.
+	 * However, server passed them through group_extra and thus fetch from it.
 	 */
-	if (extra)
+	if (group_extra)
 	{
 		/* Partial aggregates are not supported. */
-		if (extra->isPartialAgg)
+		if (group_extra->partial_partitionwise_grouping)
 			return;
 
-		fpinfo->grouped_target = extra->relTarget;
-		fpinfo->havingQual = extra->havingQual;
+		fpinfo->grouped_target = group_extra->target;
+		fpinfo->havingQual = group_extra->havingQual;
 	}
 	else
 	{
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 1611975..9a34bf9 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -109,43 +109,6 @@ typedef struct
 	int		   *tleref_to_colnum_map;
 } grouping_sets_data;
 
-/*
- * Struct for extra information passed to create_grouping_paths
- *
- * can_hash is true if hash-based grouping is possible, false otherwise.
- * can_sort is true if sort-based grouping is possible, false otherwise.
- * can_partial_agg is true if partial aggregation is possible, false otherwise.
- * partial_costs_set indicates whether agg_partial_costs and agg_final_costs
- *		have valid costs set. Both of those are computed only when partial
- *		aggregation is required.
- * agg_partial_costs gives partial aggregation costs.
- * agg_final_costs gives finalization costs.
- */
-typedef struct
-{
-	/* Data which remains constant once set. */
-	bool		can_hash;
-	bool		can_sort;
-	bool		can_partial_agg;
-	bool		partial_costs_set;
-	AggClauseCosts agg_partial_costs;
-	AggClauseCosts agg_final_costs;
-
-	/*
-	 * Data which depends upon the input and grouping relations and hence may
-	 * change across partitioning hierarchy
-	 */
-	bool		consider_parallel;	/* It probably remains same across
-									 * partitioning hierarchy. But double
-									 * check.
-									 */
-	bool		try_parallel_aggregation;
-	bool		partitionwise_grouping;
-	bool		partial_partitionwise_grouping;
-	Node	   *havingQual;
-	PathTarget *target;
-} GroupPathExtraData;
-
 /* Local functions */
 static Node *preprocess_expression(PlannerInfo *root, Node *expr, int kind);
 static void preprocess_qual_conditions(PlannerInfo *root, Node *jtnode);
@@ -3942,8 +3905,7 @@ populate_grouping_rels_with_paths(PlannerInfo *root,
 	if (extra->try_parallel_aggregation)
 		add_paths_to_partial_grouping_rel(root, input_rel,
 										  partially_grouped_rel, gd, extra,
-										  true,
-										  extra->partial_partitionwise_grouping);
+										  true, is_partial_agg);
 
 	/*
 	 * Now generate non-parallel paths, partial or full aggregation paths as
diff --git a/src/include/foreign/fdwapi.h b/src/include/foreign/fdwapi.h
index 1c2b1fc..6d94c73 100644
--- a/src/include/foreign/fdwapi.h
+++ b/src/include/foreign/fdwapi.h
@@ -63,7 +63,7 @@ typedef void (*GetForeignUpperPaths_function) (PlannerInfo *root,
 											   UpperRelationKind stage,
 											   RelOptInfo *input_rel,
 											   RelOptInfo *output_rel,
-											   OtherUpperPathExtraData *extra);
+											   GroupPathExtraData *group_extra);
 
 typedef void (*AddForeignUpdateTargets_function) (Query *parsetree,
 												  RangeTblEntry *target_rte,
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index 113e835..55edd7c 100644
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -2296,19 +2296,41 @@ typedef struct JoinPathExtraData
 } JoinPathExtraData;
 
 /*
- * Struct for extra information passed to subroutines involving path creation
- * for other upper rels.
+ * Struct for extra information passed to create_grouping_paths
  *
- * relTarget is the PathTarget for this upper rel
- * isPartialAgg is true if we are creating a partial aggregation path
- * havingQual is the quals applied to this upper rel
+ * can_hash is true if hash-based grouping is possible, false otherwise.
+ * can_sort is true if sort-based grouping is possible, false otherwise.
+ * can_partial_agg is true if partial aggregation is possible, false otherwise.
+ * partial_costs_set indicates whether agg_partial_costs and agg_final_costs
+ *		have valid costs set. Both of those are computed only when partial
+ *		aggregation is required.
+ * agg_partial_costs gives partial aggregation costs.
+ * agg_final_costs gives finalization costs.
  */
 typedef struct
 {
-	PathTarget *relTarget;
-	bool		isPartialAgg;
+	/* Data which remains constant once set. */
+	bool		can_hash;
+	bool		can_sort;
+	bool		can_partial_agg;
+	bool		partial_costs_set;
+	AggClauseCosts agg_partial_costs;
+	AggClauseCosts agg_final_costs;
+
+	/*
+	 * Data which depends upon the input and grouping relations and hence may
+	 * change across partitioning hierarchy
+	 */
+	bool		consider_parallel;	/* It probably remains same across
+									 * partitioning hierarchy. But double
+									 * check.
+									 */
+	bool		try_parallel_aggregation;
+	bool		partitionwise_grouping;
+	bool		partial_partitionwise_grouping;
 	Node	   *havingQual;
-} OtherUpperPathExtraData;
+	PathTarget *target;
+} GroupPathExtraData;
 
 /*
  * For speed reasons, cost estimation for join paths is performed in two