On Wed, Mar 28, 2018 at 7:21 PM, Ashutosh Bapat <
ashutosh.ba...@enterprisedb.com> wrote:
> On Tue, Mar 27, 2018 at 2:43 PM, Jeevan Chalke
> <jeevan.cha...@enterprisedb.com> wrote:
>
> > I am not sure on what we should Assetrt here. Note that we end-up here
> only
> > when doing grouping, and thus I don't think we need any Assert here.
> > Let me know if I missed anything.
>
> Since we are just checking whether it's an upper relation and directly
> using root->upper_targets[UPPERREL_GROUP_AGG], I thought we could add
> an assert to verify that it's really the grouping rel we are dealing
> with. But I guess, we can't really check that from given relation. But
> then for a grouped rel we can get its target from RelOptInfo. So, we
> shouldn't need to root->upper_targets[UPPERREL_GROUP_AGG]. Am I
> missing something? For other upper relations we do not set the target
> yet but then we could assert that there exists one in the grouped
> relation.
>
Yes. We fetch target from the grouped_rel itself.
Added Assert() per out off-list discussion.
>
> >
> >>
> >>
> >> - get_agg_clause_costs(root, (Node *)
> >> root->parse->havingQual,
> >> + get_agg_clause_costs(root, fpinfo->havingQual,
> >> AGGSPLIT_SIMPLE, &aggcosts);
> >> }
> >> Should we pass agg costs as well through GroupPathExtraData to avoid
> >> calculating it again in this function?
> >
> >
> > Adding an extra member in GroupPathExtraData just for FDW does not look
> good
> > to me.
> > But yes, if we do that, then we can save this calculation.
> > Let me know if its OK to have an extra member for just FDW use, will
> prepare
> > a separate patch for that.
>
> I think that should be fine. A separate patch would be good, so that a
> committer can decide whether or not to include it.
>
Attached patch 0003 for this.
>
>
> >>
> >> + Node *havingQual;
> >> I am wondering whether we could use remote_conds member for storing
> this.
> >
> >
> > This havingQual is later checked for shippability and classified into
> > pushable and non-pushable quals and stored in remote_conds and
> local_conds
> > respectively.
> > Storing it directly in remote_conds and then splitting it does not look
> good
> > to me.
> > Also, remote_conds is list of RestrictInfo nodes whereas havingQual is
> not.
> > So using that for storing havingQual does not make sense. So better to
> have
> > a separate member in PgFdwRelationInfo.
>
> Ah sorry, I was wrong about remote_conds. remote_conds and local_conds
> are basically the conditions on the relation being pushed down.
> havingQuals are conditions on a grouped relation so treating them like
> baserestrictinfo or join conditions looks more straight forward,
> rather than having a separate member in PgFdwRelationInfo. So, remote
> havingQuals go into remote_conds and local havingQuals go to
> local_conds.
>
OK. Agree.
In this version, I have not added anything in PgFdwRelationInfo.
Having qual is needed at two places; (1) in foreign_grouping_ok() to check
shippability, so passed this translated HAVING qual as a parameter to it,
and (2) estimating aggregates costs in estimate_path_cost_size(); there we
can use havingQual from root itself as costs won't change for parent and
child.
Thus no need of storing a havingQual in PgFdwRelationInfo.
Thanks
--
> Best Wishes,
> Ashutosh Bapat
> EnterpriseDB Corporation
> The Postgres Database Company
>
--
Jeevan Chalke
Technical Architect, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
From 761d46b1b255a7521529bbe7d1b128c9d79d6b4e Mon Sep 17 00:00:00 2001
From: Jeevan Chalke <jeevan.cha...@enterprisedb.com>
Date: Tue, 27 Mar 2018 14:23:00 +0530
Subject: [PATCH 1/3] Remove target from GroupPathExtraData, instead fetch it
from grouped_rel.
---
src/backend/optimizer/plan/planner.c | 4 +---
src/include/nodes/relation.h | 2 --
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index a19f5d0..d4acde6 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -3734,7 +3734,6 @@ create_grouping_paths(PlannerInfo *root,
flags |= GROUPING_CAN_PARTIAL_AGG;
extra.flags = flags;
- extra.target = target;
extra.target_parallel_safe = target_parallel_safe;
extra.havingQual = parse->havingQual;
extra.targetList = parse->targetList;
@@ -6909,7 +6908,7 @@ create_partitionwise_grouping_paths(PlannerInfo *root,
int cnt_parts;
List *grouped_live_children = NIL;
List *partially_grouped_live_children = NIL;
- PathTarget *target = extra->target;
+ PathTarget *target = grouped_rel->reltarget;
Assert(patype != PARTITIONWISE_AGGREGATE_NONE);
Assert(patype != PARTITIONWISE_AGGREGATE_PARTIAL ||
@@ -6943,7 +6942,6 @@ create_partitionwise_grouping_paths(PlannerInfo *root,
adjust_appendrel_attrs(root,
(Node *) target->exprs,
nappinfos, appinfos);
- child_extra.target = child_target;
/* Translate havingQual and targetList. */
child_extra.havingQual = (Node *)
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index abbbda9..2b4f773 100644
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -2340,7 +2340,6 @@ typedef enum
* have been initialized.
* agg_partial_costs gives partial aggregation costs.
* agg_final_costs gives finalization costs.
- * target is the PathTarget to be used while creating paths.
* target_parallel_safe is true if target is parallel safe.
* havingQual gives list of quals to be applied after aggregation.
* targetList gives list of columns to be projected.
@@ -2355,7 +2354,6 @@ typedef struct
AggClauseCosts agg_final_costs;
/* Data which may differ across partitions. */
- PathTarget *target;
bool target_parallel_safe;
Node *havingQual;
List *targetList;
--
1.8.3.1
From bdf615e1d8d06fc4b81ce80f538d61725ce04235 Mon Sep 17 00:00:00 2001
From: Jeevan Chalke <jeevan.cha...@enterprisedb.com>
Date: Tue, 27 Mar 2018 14:23:34 +0530
Subject: [PATCH 2/3] Teach postgres_fdw to push aggregates for child relations
too.
GetForeignUpperPaths() now takes an extra void parameter which will
be used to pass any additional details required to create an upper
path at the remote server. However, we support only grouping over
remote server today and thus it passes grouping specific details i.e.
GroupPathExtraData, NULL otherwise.
Since we don't know how to get a partially aggregated result from a
remote server, only full aggregation is pushed on the remote server.
---
contrib/postgres_fdw/expected/postgres_fdw.out | 132 +++++++++++++++++++++++++
contrib/postgres_fdw/postgres_fdw.c | 57 ++++++++---
contrib/postgres_fdw/sql/postgres_fdw.sql | 51 ++++++++++
doc/src/sgml/fdwhandler.sgml | 8 +-
src/backend/optimizer/plan/createplan.c | 2 +-
src/backend/optimizer/plan/planner.c | 29 +++---
src/backend/optimizer/prep/prepunion.c | 2 +-
src/include/foreign/fdwapi.h | 3 +-
src/include/optimizer/planner.h | 3 +-
9 files changed, 254 insertions(+), 33 deletions(-)
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 2d6e387..a211aa9 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -7852,3 +7852,135 @@ SELECT t1.a, t1.phv, t2.b, t2.phv FROM (SELECT 't1_phv' phv, * FROM fprt1 WHERE
(14 rows)
RESET enable_partitionwise_join;
+-- ===================================================================
+-- test partitionwise aggregates
+-- ===================================================================
+CREATE TABLE pagg_tab (a int, b int, c text) PARTITION BY RANGE(a);
+CREATE TABLE pagg_tab_p1 (LIKE pagg_tab);
+CREATE TABLE pagg_tab_p2 (LIKE pagg_tab);
+CREATE TABLE pagg_tab_p3 (LIKE pagg_tab);
+INSERT INTO pagg_tab_p1 SELECT i % 30, i % 50, to_char(i/30, 'FM0000') FROM generate_series(1, 3000) i WHERE (i % 30) < 10;
+INSERT INTO pagg_tab_p2 SELECT i % 30, i % 50, to_char(i/30, 'FM0000') FROM generate_series(1, 3000) i WHERE (i % 30) < 20 and (i % 30) >= 10;
+INSERT INTO pagg_tab_p3 SELECT i % 30, i % 50, to_char(i/30, 'FM0000') FROM generate_series(1, 3000) i WHERE (i % 30) < 30 and (i % 30) >= 20;
+-- Create foreign partitions
+CREATE FOREIGN TABLE fpagg_tab_p1 PARTITION OF pagg_tab FOR VALUES FROM (0) TO (10) SERVER loopback OPTIONS (table_name 'pagg_tab_p1');
+CREATE FOREIGN TABLE fpagg_tab_p2 PARTITION OF pagg_tab FOR VALUES FROM (10) TO (20) SERVER loopback OPTIONS (table_name 'pagg_tab_p2');;
+CREATE FOREIGN TABLE fpagg_tab_p3 PARTITION OF pagg_tab FOR VALUES FROM (20) TO (30) SERVER loopback OPTIONS (table_name 'pagg_tab_p3');;
+ANALYZE pagg_tab;
+ANALYZE fpagg_tab_p1;
+ANALYZE fpagg_tab_p2;
+ANALYZE fpagg_tab_p3;
+-- When GROUP BY clause matches with PARTITION KEY.
+-- Plan with partitionwise aggregates is disabled
+SET enable_partitionwise_aggregate TO false;
+EXPLAIN (COSTS OFF)
+SELECT a, sum(b), min(b), count(*) FROM pagg_tab GROUP BY a HAVING avg(b) < 22 ORDER BY 1;
+ QUERY PLAN
+-------------------------------------------------------
+ Sort
+ Sort Key: fpagg_tab_p1.a
+ -> HashAggregate
+ Group Key: fpagg_tab_p1.a
+ Filter: (avg(fpagg_tab_p1.b) < '22'::numeric)
+ -> Append
+ -> Foreign Scan on fpagg_tab_p1
+ -> Foreign Scan on fpagg_tab_p2
+ -> Foreign Scan on fpagg_tab_p3
+(9 rows)
+
+-- Plan with partitionwise aggregates is enabled
+SET enable_partitionwise_aggregate TO true;
+EXPLAIN (COSTS OFF)
+SELECT a, sum(b), min(b), count(*) FROM pagg_tab GROUP BY a HAVING avg(b) < 22 ORDER BY 1;
+ QUERY PLAN
+----------------------------------------------------------------------
+ Sort
+ Sort Key: fpagg_tab_p1.a
+ -> Append
+ -> Foreign Scan
+ Relations: Aggregate on (public.fpagg_tab_p1 pagg_tab)
+ -> Foreign Scan
+ Relations: Aggregate on (public.fpagg_tab_p2 pagg_tab)
+ -> Foreign Scan
+ Relations: Aggregate on (public.fpagg_tab_p3 pagg_tab)
+(9 rows)
+
+SELECT a, sum(b), min(b), count(*) FROM pagg_tab GROUP BY a HAVING avg(b) < 22 ORDER BY 1;
+ a | sum | min | count
+----+------+-----+-------
+ 0 | 2000 | 0 | 100
+ 1 | 2100 | 1 | 100
+ 10 | 2000 | 0 | 100
+ 11 | 2100 | 1 | 100
+ 20 | 2000 | 0 | 100
+ 21 | 2100 | 1 | 100
+(6 rows)
+
+-- Check with whole-row reference
+-- Should have all the columns in the target list for the given relation
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT a, count(t1) FROM pagg_tab t1 GROUP BY a HAVING avg(b) < 22 ORDER BY 1;
+ QUERY PLAN
+------------------------------------------------------------------------
+ Sort
+ Output: t1.a, (count(((t1.*)::pagg_tab)))
+ Sort Key: t1.a
+ -> Append
+ -> HashAggregate
+ Output: t1.a, count(((t1.*)::pagg_tab))
+ Group Key: t1.a
+ Filter: (avg(t1.b) < '22'::numeric)
+ -> Foreign Scan on public.fpagg_tab_p1 t1
+ Output: t1.a, t1.*, t1.b
+ Remote SQL: SELECT a, b, c FROM public.pagg_tab_p1
+ -> HashAggregate
+ Output: t1_1.a, count(((t1_1.*)::pagg_tab))
+ Group Key: t1_1.a
+ Filter: (avg(t1_1.b) < '22'::numeric)
+ -> Foreign Scan on public.fpagg_tab_p2 t1_1
+ Output: t1_1.a, t1_1.*, t1_1.b
+ Remote SQL: SELECT a, b, c FROM public.pagg_tab_p2
+ -> HashAggregate
+ Output: t1_2.a, count(((t1_2.*)::pagg_tab))
+ Group Key: t1_2.a
+ Filter: (avg(t1_2.b) < '22'::numeric)
+ -> Foreign Scan on public.fpagg_tab_p3 t1_2
+ Output: t1_2.a, t1_2.*, t1_2.b
+ Remote SQL: SELECT a, b, c FROM public.pagg_tab_p3
+(25 rows)
+
+SELECT a, count(t1) FROM pagg_tab t1 GROUP BY a HAVING avg(b) < 22 ORDER BY 1;
+ a | count
+----+-------
+ 0 | 100
+ 1 | 100
+ 10 | 100
+ 11 | 100
+ 20 | 100
+ 21 | 100
+(6 rows)
+
+-- When GROUP BY clause does not match with PARTITION KEY.
+EXPLAIN (COSTS OFF)
+SELECT b, avg(a), max(a), count(*) FROM pagg_tab GROUP BY b HAVING sum(a) < 700 ORDER BY 1;
+ QUERY PLAN
+------------------------------------------------------
+ Sort
+ Sort Key: fpagg_tab_p1.b
+ -> Finalize HashAggregate
+ Group Key: fpagg_tab_p1.b
+ Filter: (sum(fpagg_tab_p1.a) < 700)
+ -> Append
+ -> Partial HashAggregate
+ Group Key: fpagg_tab_p1.b
+ -> Foreign Scan on fpagg_tab_p1
+ -> Partial HashAggregate
+ Group Key: fpagg_tab_p2.b
+ -> Foreign Scan on fpagg_tab_p2
+ -> Partial HashAggregate
+ Group Key: fpagg_tab_p3.b
+ -> Foreign Scan on fpagg_tab_p3
+(15 rows)
+
+-- Clean-up
+RESET enable_partitionwise_aggregate;
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index e8a0d54..dbebbda 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -352,7 +352,8 @@ static bool postgresRecheckForeignScan(ForeignScanState *node,
static void postgresGetForeignUpperPaths(PlannerInfo *root,
UpperRelationKind stage,
RelOptInfo *input_rel,
- RelOptInfo *output_rel);
+ RelOptInfo *output_rel,
+ void *extra);
/*
* Helper functions
@@ -419,7 +420,8 @@ static void conversion_error_callback(void *arg);
static bool foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel,
JoinType jointype, RelOptInfo *outerrel, RelOptInfo *innerrel,
JoinPathExtraData *extra);
-static bool foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel);
+static bool foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel,
+ Node *havingQual);
static List *get_useful_pathkeys_for_relation(PlannerInfo *root,
RelOptInfo *rel);
static List *get_useful_ecs_for_relation(PlannerInfo *root, RelOptInfo *rel);
@@ -427,7 +429,8 @@ static void add_paths_with_pathkeys_for_rel(PlannerInfo *root, RelOptInfo *rel,
Path *epq_path);
static void add_foreign_grouping_paths(PlannerInfo *root,
RelOptInfo *input_rel,
- RelOptInfo *grouped_rel);
+ RelOptInfo *grouped_rel,
+ GroupPathExtraData *extra);
static void apply_server_options(PgFdwRelationInfo *fpinfo);
static void apply_table_options(PgFdwRelationInfo *fpinfo);
static void merge_fdw_options(PgFdwRelationInfo *fpinfo,
@@ -2775,13 +2778,19 @@ estimate_path_cost_size(PlannerInfo *root,
else if (IS_UPPER_REL(foreignrel))
{
PgFdwRelationInfo *ofpinfo;
- PathTarget *ptarget = root->upper_targets[UPPERREL_GROUP_AGG];
+ PathTarget *ptarget = foreignrel->reltarget;
AggClauseCosts aggcosts;
double input_rows;
int numGroupCols;
double numGroups = 1;
/*
+ * For grouping and/or aggregation, we do set path target in
+ * grouped_rel's reltarget. Thus we must have it.
+ */
+ Assert(ptarget);
+
+ /*
* This cost model is mixture of costing done for sorted and
* hashed aggregates in cost_agg(). We are not sure which
* strategy will be considered at remote side, thus for
@@ -2805,6 +2814,13 @@ estimate_path_cost_size(PlannerInfo *root,
{
get_agg_clause_costs(root, (Node *) fpinfo->grouped_tlist,
AGGSPLIT_SIMPLE, &aggcosts);
+
+ /*
+ * Cost of aggregates within the HAVING qual will remain same
+ * for both parent and a child. Thus, in case of child upper
+ * rel, it is not necessary to consider translated HAVING qual
+ * here. Hence use having qual from the root itself.
+ */
get_agg_clause_costs(root, (Node *) root->parse->havingQual,
AGGSPLIT_SIMPLE, &aggcosts);
}
@@ -5017,11 +5033,12 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
* this function to PgFdwRelationInfo of the input relation.
*/
static bool
-foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel)
+foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel,
+ Node *havingQual)
{
Query *query = root->parse;
- PathTarget *grouping_target = root->upper_targets[UPPERREL_GROUP_AGG];
PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) grouped_rel->fdw_private;
+ PathTarget *grouping_target = grouped_rel->reltarget;
PgFdwRelationInfo *ofpinfo;
List *aggvars;
ListCell *lc;
@@ -5131,11 +5148,11 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel)
* Classify the pushable and non-pushable HAVING clauses and save them in
* remote_conds and local_conds of the grouped rel's fpinfo.
*/
- if (root->hasHavingQual && query->havingQual)
+ if (havingQual)
{
ListCell *lc;
- foreach(lc, (List *) query->havingQual)
+ foreach(lc, (List *) havingQual)
{
Expr *expr = (Expr *) lfirst(lc);
RestrictInfo *rinfo;
@@ -5232,7 +5249,8 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel)
*/
static void
postgresGetForeignUpperPaths(PlannerInfo *root, UpperRelationKind stage,
- RelOptInfo *input_rel, RelOptInfo *output_rel)
+ RelOptInfo *input_rel, RelOptInfo *output_rel,
+ void *extra)
{
PgFdwRelationInfo *fpinfo;
@@ -5252,7 +5270,8 @@ postgresGetForeignUpperPaths(PlannerInfo *root, UpperRelationKind stage,
fpinfo->pushdown_safe = false;
output_rel->fdw_private = fpinfo;
- add_foreign_grouping_paths(root, input_rel, output_rel);
+ add_foreign_grouping_paths(root, input_rel, output_rel,
+ (GroupPathExtraData *) extra);
}
/*
@@ -5264,13 +5283,13 @@ postgresGetForeignUpperPaths(PlannerInfo *root, UpperRelationKind stage,
*/
static void
add_foreign_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
- RelOptInfo *grouped_rel)
+ RelOptInfo *grouped_rel,
+ GroupPathExtraData *extra)
{
Query *parse = root->parse;
PgFdwRelationInfo *ifpinfo = input_rel->fdw_private;
PgFdwRelationInfo *fpinfo = grouped_rel->fdw_private;
ForeignPath *grouppath;
- PathTarget *grouping_target;
double rows;
int width;
Cost startup_cost;
@@ -5281,7 +5300,8 @@ add_foreign_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
!root->hasHavingQual)
return;
- grouping_target = root->upper_targets[UPPERREL_GROUP_AGG];
+ Assert(extra->patype == PARTITIONWISE_AGGREGATE_NONE ||
+ extra->patype == PARTITIONWISE_AGGREGATE_FULL);
/* save the input_rel as outerrel in fpinfo */
fpinfo->outerrel = input_rel;
@@ -5295,8 +5315,13 @@ add_foreign_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
fpinfo->user = ifpinfo->user;
merge_fdw_options(fpinfo, ifpinfo, NULL);
- /* Assess if it is safe to push down aggregation and grouping. */
- if (!foreign_grouping_ok(root, grouped_rel))
+ /*
+ * Assess if it is safe to push down aggregation and grouping.
+ *
+ * Use HAVING qual from extra. In case of child partition, it will have
+ * translated Vars.
+ */
+ if (!foreign_grouping_ok(root, grouped_rel, extra->havingQual))
return;
/* Estimate the cost of push down */
@@ -5312,7 +5337,7 @@ add_foreign_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
/* Create and add foreign path to the grouping relation. */
grouppath = create_foreignscan_path(root,
grouped_rel,
- grouping_target,
+ grouped_rel->reltarget,
rows,
startup_cost,
total_cost,
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 4d2e43c..cf32be4 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -1932,3 +1932,54 @@ SELECT t1.a, t1.phv, t2.b, t2.phv FROM (SELECT 't1_phv' phv, * FROM fprt1 WHERE
SELECT t1.a, t1.phv, t2.b, t2.phv FROM (SELECT 't1_phv' phv, * FROM fprt1 WHERE a % 25 = 0) t1 FULL JOIN (SELECT 't2_phv' phv, * FROM fprt2 WHERE b % 25 = 0) t2 ON (t1.a = t2.b) ORDER BY t1.a, t2.b;
RESET enable_partitionwise_join;
+
+
+-- ===================================================================
+-- test partitionwise aggregates
+-- ===================================================================
+
+CREATE TABLE pagg_tab (a int, b int, c text) PARTITION BY RANGE(a);
+
+CREATE TABLE pagg_tab_p1 (LIKE pagg_tab);
+CREATE TABLE pagg_tab_p2 (LIKE pagg_tab);
+CREATE TABLE pagg_tab_p3 (LIKE pagg_tab);
+
+INSERT INTO pagg_tab_p1 SELECT i % 30, i % 50, to_char(i/30, 'FM0000') FROM generate_series(1, 3000) i WHERE (i % 30) < 10;
+INSERT INTO pagg_tab_p2 SELECT i % 30, i % 50, to_char(i/30, 'FM0000') FROM generate_series(1, 3000) i WHERE (i % 30) < 20 and (i % 30) >= 10;
+INSERT INTO pagg_tab_p3 SELECT i % 30, i % 50, to_char(i/30, 'FM0000') FROM generate_series(1, 3000) i WHERE (i % 30) < 30 and (i % 30) >= 20;
+
+-- Create foreign partitions
+CREATE FOREIGN TABLE fpagg_tab_p1 PARTITION OF pagg_tab FOR VALUES FROM (0) TO (10) SERVER loopback OPTIONS (table_name 'pagg_tab_p1');
+CREATE FOREIGN TABLE fpagg_tab_p2 PARTITION OF pagg_tab FOR VALUES FROM (10) TO (20) SERVER loopback OPTIONS (table_name 'pagg_tab_p2');;
+CREATE FOREIGN TABLE fpagg_tab_p3 PARTITION OF pagg_tab FOR VALUES FROM (20) TO (30) SERVER loopback OPTIONS (table_name 'pagg_tab_p3');;
+
+ANALYZE pagg_tab;
+ANALYZE fpagg_tab_p1;
+ANALYZE fpagg_tab_p2;
+ANALYZE fpagg_tab_p3;
+
+-- When GROUP BY clause matches with PARTITION KEY.
+-- Plan with partitionwise aggregates is disabled
+SET enable_partitionwise_aggregate TO false;
+EXPLAIN (COSTS OFF)
+SELECT a, sum(b), min(b), count(*) FROM pagg_tab GROUP BY a HAVING avg(b) < 22 ORDER BY 1;
+
+-- Plan with partitionwise aggregates is enabled
+SET enable_partitionwise_aggregate TO true;
+EXPLAIN (COSTS OFF)
+SELECT a, sum(b), min(b), count(*) FROM pagg_tab GROUP BY a HAVING avg(b) < 22 ORDER BY 1;
+SELECT a, sum(b), min(b), count(*) FROM pagg_tab GROUP BY a HAVING avg(b) < 22 ORDER BY 1;
+
+-- Check with whole-row reference
+-- Should have all the columns in the target list for the given relation
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT a, count(t1) FROM pagg_tab t1 GROUP BY a HAVING avg(b) < 22 ORDER BY 1;
+SELECT a, count(t1) FROM pagg_tab t1 GROUP BY a HAVING avg(b) < 22 ORDER BY 1;
+
+-- When GROUP BY clause does not match with PARTITION KEY.
+EXPLAIN (COSTS OFF)
+SELECT b, avg(a), max(a), count(*) FROM pagg_tab GROUP BY b HAVING sum(a) < 700 ORDER BY 1;
+
+
+-- Clean-up
+RESET enable_partitionwise_aggregate;
diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml
index 0ed3a47..25915a2 100644
--- a/doc/src/sgml/fdwhandler.sgml
+++ b/doc/src/sgml/fdwhandler.sgml
@@ -359,7 +359,8 @@ void
GetForeignUpperPaths(PlannerInfo *root,
UpperRelationKind stage,
RelOptInfo *input_rel,
- RelOptInfo *output_rel);
+ RelOptInfo *output_rel,
+ void *extra);
</programlisting>
Create possible access paths for <firstterm>upper relation</firstterm> processing,
which is the planner's term for all post-scan/join query processing, such
@@ -379,7 +380,10 @@ GetForeignUpperPaths(PlannerInfo *root,
currently being considered. <literal>output_rel</literal> is the upper relation
that should receive paths representing computation of this step,
and <literal>input_rel</literal> is the relation representing the input to this
- step. (Note that <structname>ForeignPath</structname> paths added
+ step. <literal>extra</literal> parameter provides additional details which may
+ be needed for the paths creation, like child details in case of partitioning.
+ The details passed through this parameter is depends on the <literal>stage</literal>
+ parameter. (Note that <structname>ForeignPath</structname> paths added
to <literal>output_rel</literal> would typically not have any direct dependency
on paths of the <literal>input_rel</literal>, since their processing is expected
to be done externally. However, examining paths previously generated for
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 8b4f031..8bd15c9 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -3520,7 +3520,7 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
* 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.
*/
- if (IS_UPPER_REL(rel))
+ if (rel->reloptkind == RELOPT_UPPER_REL)
scan_plan->fs_relids = root->all_baserels;
else
scan_plan->fs_relids = best_path->path.parent->relids;
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index d4acde6..c5fef61 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -2208,12 +2208,13 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
if (final_rel->fdwroutine &&
final_rel->fdwroutine->GetForeignUpperPaths)
final_rel->fdwroutine->GetForeignUpperPaths(root, UPPERREL_FINAL,
- current_rel, final_rel);
+ current_rel, final_rel,
+ NULL);
/* Let extensions possibly add some more paths */
if (create_upper_paths_hook)
(*create_upper_paths_hook) (root, UPPERREL_FINAL,
- current_rel, final_rel);
+ current_rel, final_rel, NULL);
/* Note: currently, we leave it to callers to do set_cheapest() */
}
@@ -4026,12 +4027,14 @@ create_ordinary_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
if (grouped_rel->fdwroutine &&
grouped_rel->fdwroutine->GetForeignUpperPaths)
grouped_rel->fdwroutine->GetForeignUpperPaths(root, UPPERREL_GROUP_AGG,
- input_rel, grouped_rel);
+ input_rel, grouped_rel,
+ extra);
/* Let extensions possibly add some more paths */
if (create_upper_paths_hook)
(*create_upper_paths_hook) (root, UPPERREL_GROUP_AGG,
- input_rel, grouped_rel);
+ input_rel, grouped_rel,
+ extra);
}
/*
@@ -4463,12 +4466,13 @@ create_window_paths(PlannerInfo *root,
if (window_rel->fdwroutine &&
window_rel->fdwroutine->GetForeignUpperPaths)
window_rel->fdwroutine->GetForeignUpperPaths(root, UPPERREL_WINDOW,
- input_rel, window_rel);
+ input_rel, window_rel,
+ NULL);
/* Let extensions possibly add some more paths */
if (create_upper_paths_hook)
(*create_upper_paths_hook) (root, UPPERREL_WINDOW,
- input_rel, window_rel);
+ input_rel, window_rel, NULL);
/* Now choose the best path(s) */
set_cheapest(window_rel);
@@ -4767,12 +4771,13 @@ create_distinct_paths(PlannerInfo *root,
if (distinct_rel->fdwroutine &&
distinct_rel->fdwroutine->GetForeignUpperPaths)
distinct_rel->fdwroutine->GetForeignUpperPaths(root, UPPERREL_DISTINCT,
- input_rel, distinct_rel);
+ input_rel, distinct_rel,
+ NULL);
/* Let extensions possibly add some more paths */
if (create_upper_paths_hook)
(*create_upper_paths_hook) (root, UPPERREL_DISTINCT,
- input_rel, distinct_rel);
+ input_rel, distinct_rel, NULL);
/* Now choose the best path(s) */
set_cheapest(distinct_rel);
@@ -4910,12 +4915,13 @@ create_ordered_paths(PlannerInfo *root,
if (ordered_rel->fdwroutine &&
ordered_rel->fdwroutine->GetForeignUpperPaths)
ordered_rel->fdwroutine->GetForeignUpperPaths(root, UPPERREL_ORDERED,
- input_rel, ordered_rel);
+ input_rel, ordered_rel,
+ NULL);
/* Let extensions possibly add some more paths */
if (create_upper_paths_hook)
(*create_upper_paths_hook) (root, UPPERREL_ORDERED,
- input_rel, ordered_rel);
+ input_rel, ordered_rel, NULL);
/*
* No need to bother with set_cheapest here; grouping_planner does not
@@ -6696,7 +6702,8 @@ create_partial_grouping_paths(PlannerInfo *root,
fdwroutine->GetForeignUpperPaths(root,
UPPERREL_PARTIAL_GROUP_AGG,
- input_rel, partially_grouped_rel);
+ input_rel, partially_grouped_rel,
+ extra);
}
return partially_grouped_rel;
diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c
index 6e510f9..5236ab3 100644
--- a/src/backend/optimizer/prep/prepunion.c
+++ b/src/backend/optimizer/prep/prepunion.c
@@ -1032,7 +1032,7 @@ postprocess_setop_rel(PlannerInfo *root, RelOptInfo *rel)
*/
if (create_upper_paths_hook)
(*create_upper_paths_hook) (root, UPPERREL_SETOP,
- NULL, rel);
+ NULL, rel, NULL);
/* Select cheapest path */
set_cheapest(rel);
diff --git a/src/include/foreign/fdwapi.h b/src/include/foreign/fdwapi.h
index e88fee3..ea83c7b 100644
--- a/src/include/foreign/fdwapi.h
+++ b/src/include/foreign/fdwapi.h
@@ -62,7 +62,8 @@ typedef void (*GetForeignJoinPaths_function) (PlannerInfo *root,
typedef void (*GetForeignUpperPaths_function) (PlannerInfo *root,
UpperRelationKind stage,
RelOptInfo *input_rel,
- RelOptInfo *output_rel);
+ RelOptInfo *output_rel,
+ void *extra);
typedef void (*AddForeignUpdateTargets_function) (Query *parsetree,
RangeTblEntry *target_rte,
diff --git a/src/include/optimizer/planner.h b/src/include/optimizer/planner.h
index 0d8b88d..07a3bc0 100644
--- a/src/include/optimizer/planner.h
+++ b/src/include/optimizer/planner.h
@@ -28,7 +28,8 @@ extern PGDLLIMPORT planner_hook_type planner_hook;
typedef void (*create_upper_paths_hook_type) (PlannerInfo *root,
UpperRelationKind stage,
RelOptInfo *input_rel,
- RelOptInfo *output_rel);
+ RelOptInfo *output_rel,
+ void *extra);
extern PGDLLIMPORT create_upper_paths_hook_type create_upper_paths_hook;
--
1.8.3.1
From f6569b886cd4db9b7bb1de65b530a692dd6dc734 Mon Sep 17 00:00:00 2001
From: Jeevan Chalke <jeevan.cha...@enterprisedb.com>
Date: Thu, 29 Mar 2018 18:04:05 +0530
Subject: [PATCH 3/3] Add agg_costs in GroupPathExtraData so that FDW can use
it.
We do pass GroupPathExtraData to postgres_fdw, thus add agg_costs
too in the struct so that we can use these cost estimates in
postgres_fdw too avoiding recalculations.
In passing, updated couple of function signatures where we were
passing both agg_costs and "extra" so that it just passes the
"extra" parameter.
---
contrib/postgres_fdw/postgres_fdw.c | 43 +++++++++++++-----------------------
src/backend/optimizer/plan/planner.c | 31 ++++++++++++--------------
src/include/nodes/relation.h | 3 +++
3 files changed, 32 insertions(+), 45 deletions(-)
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index dbebbda..6d85569 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -362,6 +362,7 @@ static void estimate_path_cost_size(PlannerInfo *root,
RelOptInfo *foreignrel,
List *param_join_conds,
List *pathkeys,
+ const AggClauseCosts *agg_costs,
double *p_rows, int *p_width,
Cost *p_startup_cost, Cost *p_total_cost);
static void get_remote_estimate(const char *sql,
@@ -614,7 +615,7 @@ postgresGetForeignRelSize(PlannerInfo *root,
* values in fpinfo so we don't need to do it again to generate the
* basic foreign path.
*/
- estimate_path_cost_size(root, baserel, NIL, NIL,
+ estimate_path_cost_size(root, baserel, NIL, NIL, NULL,
&fpinfo->rows, &fpinfo->width,
&fpinfo->startup_cost, &fpinfo->total_cost);
@@ -645,7 +646,7 @@ postgresGetForeignRelSize(PlannerInfo *root,
set_baserel_size_estimates(root, baserel);
/* Fill in basically-bogus cost estimates for use later. */
- estimate_path_cost_size(root, baserel, NIL, NIL,
+ estimate_path_cost_size(root, baserel, NIL, NIL, NULL,
&fpinfo->rows, &fpinfo->width,
&fpinfo->startup_cost, &fpinfo->total_cost);
}
@@ -1076,7 +1077,7 @@ postgresGetForeignPaths(PlannerInfo *root,
/* Get a cost estimate from the remote */
estimate_path_cost_size(root, baserel,
- param_info->ppi_clauses, NIL,
+ param_info->ppi_clauses, NIL, NULL,
&rows, &width,
&startup_cost, &total_cost);
@@ -2588,6 +2589,7 @@ estimate_path_cost_size(PlannerInfo *root,
RelOptInfo *foreignrel,
List *param_join_conds,
List *pathkeys,
+ const AggClauseCosts *agg_costs,
double *p_rows, int *p_width,
Cost *p_startup_cost, Cost *p_total_cost)
{
@@ -2779,7 +2781,6 @@ estimate_path_cost_size(PlannerInfo *root,
{
PgFdwRelationInfo *ofpinfo;
PathTarget *ptarget = foreignrel->reltarget;
- AggClauseCosts aggcosts;
double input_rows;
int numGroupCols;
double numGroups = 1;
@@ -2808,23 +2809,6 @@ estimate_path_cost_size(PlannerInfo *root,
input_rows = ofpinfo->rows;
width = ofpinfo->width;
- /* Collect statistics about aggregates for estimating costs. */
- MemSet(&aggcosts, 0, sizeof(AggClauseCosts));
- if (root->parse->hasAggs)
- {
- get_agg_clause_costs(root, (Node *) fpinfo->grouped_tlist,
- AGGSPLIT_SIMPLE, &aggcosts);
-
- /*
- * Cost of aggregates within the HAVING qual will remain same
- * for both parent and a child. Thus, in case of child upper
- * rel, it is not necessary to consider translated HAVING qual
- * here. Hence use having qual from the root itself.
- */
- get_agg_clause_costs(root, (Node *) root->parse->havingQual,
- AGGSPLIT_SIMPLE, &aggcosts);
- }
-
/* Get number of grouping columns and possible number of groups */
numGroupCols = list_length(root->parse->groupClause);
numGroups = estimate_num_groups(root,
@@ -2838,6 +2822,9 @@ estimate_path_cost_size(PlannerInfo *root,
*/
rows = retrieved_rows = numGroups;
+ /* agg_costs should not be null while grouping. */
+ Assert(agg_costs);
+
/*-----
* Startup cost includes:
* 1. Startup cost for underneath input * relation
@@ -2846,8 +2833,8 @@ estimate_path_cost_size(PlannerInfo *root,
*-----
*/
startup_cost = ofpinfo->rel_startup_cost;
- startup_cost += aggcosts.transCost.startup;
- startup_cost += aggcosts.transCost.per_tuple * input_rows;
+ startup_cost += agg_costs->transCost.startup;
+ startup_cost += agg_costs->transCost.per_tuple * input_rows;
startup_cost += (cpu_operator_cost * numGroupCols) * input_rows;
startup_cost += ptarget->cost.startup;
@@ -2859,7 +2846,7 @@ estimate_path_cost_size(PlannerInfo *root,
*-----
*/
run_cost = ofpinfo->rel_total_cost - ofpinfo->rel_startup_cost;
- run_cost += aggcosts.finalCost * numGroups;
+ run_cost += agg_costs->finalCost * numGroups;
run_cost += cpu_tuple_cost * numGroups;
run_cost += ptarget->cost.per_tuple * numGroups;
}
@@ -4761,7 +4748,7 @@ add_paths_with_pathkeys_for_rel(PlannerInfo *root, RelOptInfo *rel,
List *useful_pathkeys = lfirst(lc);
Path *sorted_epq_path;
- estimate_path_cost_size(root, rel, NIL, useful_pathkeys,
+ estimate_path_cost_size(root, rel, NIL, useful_pathkeys, NULL,
&rows, &width, &startup_cost, &total_cost);
/*
@@ -4993,7 +4980,7 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
extra->sjinfo);
/* Estimate costs for bare join relation */
- estimate_path_cost_size(root, joinrel, NIL, NIL, &rows,
+ estimate_path_cost_size(root, joinrel, NIL, NIL, NULL, &rows,
&width, &startup_cost, &total_cost);
/* Now update this information in the joinrel */
joinrel->rows = rows;
@@ -5325,8 +5312,8 @@ add_foreign_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
return;
/* Estimate the cost of push down */
- estimate_path_cost_size(root, grouped_rel, NIL, NIL, &rows,
- &width, &startup_cost, &total_cost);
+ estimate_path_cost_size(root, grouped_rel, NIL, NIL, extra->agg_costs,
+ &rows, &width, &startup_cost, &total_cost);
/* Now update this information in the fpinfo */
fpinfo->rows = rows;
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index c5fef61..32b2dc8 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -152,7 +152,6 @@ static RelOptInfo *make_grouping_rel(PlannerInfo *root, RelOptInfo *input_rel,
static void create_ordinary_grouping_paths(PlannerInfo *root,
RelOptInfo *input_rel,
RelOptInfo *grouped_rel,
- const AggClauseCosts *agg_costs,
grouping_sets_data *gd,
GroupPathExtraData *extra,
RelOptInfo **partially_grouped_rel_p);
@@ -207,7 +206,6 @@ static void adjust_paths_for_srfs(PlannerInfo *root, RelOptInfo *rel,
static void add_paths_to_grouping_rel(PlannerInfo *root, RelOptInfo *input_rel,
RelOptInfo *grouped_rel,
RelOptInfo *partially_grouped_rel,
- const AggClauseCosts *agg_costs,
grouping_sets_data *gd,
double dNumGroups,
GroupPathExtraData *extra);
@@ -229,7 +227,6 @@ static void create_partitionwise_grouping_paths(PlannerInfo *root,
RelOptInfo *input_rel,
RelOptInfo *grouped_rel,
RelOptInfo *partially_grouped_rel,
- const AggClauseCosts *agg_costs,
grouping_sets_data *gd,
PartitionwiseAggregateType patype,
GroupPathExtraData *extra);
@@ -3689,6 +3686,12 @@ create_grouping_paths(PlannerInfo *root,
GroupPathExtraData extra;
/*
+ * Set agg_costs into the extra so that other routines like FDW can use
+ * it directly rather than computing it again.
+ */
+ extra.agg_costs = agg_costs;
+
+ /*
* Determine whether it's possible to perform sort-based
* implementations of grouping. (Note that if groupClause is empty,
* grouping_is_sortable() is trivially true, and all the
@@ -3751,9 +3754,8 @@ create_grouping_paths(PlannerInfo *root,
else
extra.patype = PARTITIONWISE_AGGREGATE_NONE;
- create_ordinary_grouping_paths(root, input_rel, grouped_rel,
- agg_costs, gd, &extra,
- &partially_grouped_rel);
+ create_ordinary_grouping_paths(root, input_rel, grouped_rel, gd,
+ &extra, &partially_grouped_rel);
}
set_cheapest(grouped_rel);
@@ -3907,9 +3909,7 @@ create_degenerate_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
*/
static void
create_ordinary_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
- RelOptInfo *grouped_rel,
- const AggClauseCosts *agg_costs,
- grouping_sets_data *gd,
+ RelOptInfo *grouped_rel, grouping_sets_data *gd,
GroupPathExtraData *extra,
RelOptInfo **partially_grouped_rel_p)
{
@@ -3979,8 +3979,8 @@ create_ordinary_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
/* Apply partitionwise aggregation technique, if possible. */
if (patype != PARTITIONWISE_AGGREGATE_NONE)
create_partitionwise_grouping_paths(root, input_rel, grouped_rel,
- partially_grouped_rel, agg_costs,
- gd, patype, extra);
+ partially_grouped_rel, gd, patype,
+ extra);
/* If we are doing partial aggregation only, return. */
if (extra->patype == PARTITIONWISE_AGGREGATE_PARTIAL)
@@ -4010,8 +4010,7 @@ create_ordinary_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
/* Build final grouping paths */
add_paths_to_grouping_rel(root, input_rel, grouped_rel,
- partially_grouped_rel, agg_costs, gd,
- dNumGroups, extra);
+ partially_grouped_rel, gd, dNumGroups, extra);
/* Give a helpful error if we failed to find any implementation */
if (grouped_rel->pathlist == NIL)
@@ -6189,7 +6188,6 @@ static void
add_paths_to_grouping_rel(PlannerInfo *root, RelOptInfo *input_rel,
RelOptInfo *grouped_rel,
RelOptInfo *partially_grouped_rel,
- const AggClauseCosts *agg_costs,
grouping_sets_data *gd, double dNumGroups,
GroupPathExtraData *extra)
{
@@ -6199,6 +6197,7 @@ add_paths_to_grouping_rel(PlannerInfo *root, RelOptInfo *input_rel,
bool can_hash = (extra->flags & GROUPING_CAN_USE_HASH) != 0;
bool can_sort = (extra->flags & GROUPING_CAN_USE_SORT) != 0;
List *havingQual = (List *) extra->havingQual;
+ const AggClauseCosts *agg_costs = extra->agg_costs;
AggClauseCosts *agg_final_costs = &extra->agg_final_costs;
if (can_sort)
@@ -6906,7 +6905,6 @@ create_partitionwise_grouping_paths(PlannerInfo *root,
RelOptInfo *input_rel,
RelOptInfo *grouped_rel,
RelOptInfo *partially_grouped_rel,
- const AggClauseCosts *agg_costs,
grouping_sets_data *gd,
PartitionwiseAggregateType patype,
GroupPathExtraData *extra)
@@ -7006,8 +7004,7 @@ create_partitionwise_grouping_paths(PlannerInfo *root,
/* Create grouping paths for this child relation. */
create_ordinary_grouping_paths(root, child_input_rel,
- child_grouped_rel,
- agg_costs, gd, &child_extra,
+ child_grouped_rel, gd, &child_extra,
&child_partially_grouped_rel);
if (child_partially_grouped_rel)
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index 2b4f773..4ed3eb7 100644
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -2335,6 +2335,8 @@ typedef enum
/*
* Struct for extra information passed to subroutines of create_grouping_paths
*
+ * agg_costs gives cost info about all aggregates in query (in AGGSPLIT_SIMPLE
+ * mode)
* flags indicating what kinds of grouping are possible.
* partial_costs_set is true if the agg_partial_costs and agg_final_costs
* have been initialized.
@@ -2348,6 +2350,7 @@ typedef enum
typedef struct
{
/* Data which remains constant once set. */
+ const AggClauseCosts *agg_costs;
int flags;
bool partial_costs_set;
AggClauseCosts agg_partial_costs;
--
1.8.3.1
