Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit edbcbe2

Browse files
author
Etsuro Fujita
committed
postgres_fdw: Fix cost estimation for aggregate pushdown.
In commit 7012b13, which added support for aggregate pushdown in postgres_fdw, the expense of evaluating the final scan/join target computed by make_group_input_target() was not accounted for at all in costing aggregate pushdown paths with local statistics. The right fix for this would be to have a separate upper stage to adjust the final scan/join relation (see comments for apply_scanjoin_target_to_paths()); but for now, fix by adding the tlist eval cost when costing aggregate pushdown paths with local statistics. Apply this to HEAD only to avoid destabilizing existing plan choices. Author: Etsuro Fujita Reviewed-By: Antonin Houska Discussion: https://postgr.es/m/5C66A056.60007%40lab.ntt.co.jp
1 parent 47a338c commit edbcbe2

File tree

2 files changed

+17
-3
lines changed

2 files changed

+17
-3
lines changed

contrib/postgres_fdw/postgres_fdw.c

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2842,12 +2842,18 @@ estimate_path_cost_size(PlannerInfo *root,
28422842
}
28432843
else if (IS_UPPER_REL(foreignrel))
28442844
{
2845+
RelOptInfo *outerrel = fpinfo->outerrel;
28452846
PgFdwRelationInfo *ofpinfo;
28462847
AggClauseCosts aggcosts;
28472848
double input_rows;
28482849
int numGroupCols;
28492850
double numGroups = 1;
28502851

2852+
/* The upper relation should have its outer relation set */
2853+
Assert(outerrel);
2854+
/* and that outer relation should have its reltarget set */
2855+
Assert(outerrel->reltarget);
2856+
28512857
/*
28522858
* This cost model is mixture of costing done for sorted and
28532859
* hashed aggregates in cost_agg(). We are not sure which
@@ -2856,7 +2862,7 @@ estimate_path_cost_size(PlannerInfo *root,
28562862
* and all finalization and run cost are added in total_cost.
28572863
*/
28582864

2859-
ofpinfo = (PgFdwRelationInfo *) fpinfo->outerrel->fdw_private;
2865+
ofpinfo = (PgFdwRelationInfo *) outerrel->fdw_private;
28602866

28612867
/* Get rows and width from input rel */
28622868
input_rows = ofpinfo->rows;
@@ -2909,23 +2915,27 @@ estimate_path_cost_size(PlannerInfo *root,
29092915

29102916
/*-----
29112917
* Startup cost includes:
2912-
* 1. Startup cost for underneath input relation
2918+
* 1. Startup cost for underneath input relation, adjusted for
2919+
* tlist replacement by apply_scanjoin_target_to_paths()
29132920
* 2. Cost of performing aggregation, per cost_agg()
29142921
*-----
29152922
*/
29162923
startup_cost = ofpinfo->rel_startup_cost;
2924+
startup_cost += outerrel->reltarget->cost.startup;
29172925
startup_cost += aggcosts.transCost.startup;
29182926
startup_cost += aggcosts.transCost.per_tuple * input_rows;
29192927
startup_cost += aggcosts.finalCost.startup;
29202928
startup_cost += (cpu_operator_cost * numGroupCols) * input_rows;
29212929

29222930
/*-----
29232931
* Run time cost includes:
2924-
* 1. Run time cost of underneath input relation
2932+
* 1. Run time cost of underneath input relation, adjusted for
2933+
* tlist replacement by apply_scanjoin_target_to_paths()
29252934
* 2. Run time cost of performing aggregation, per cost_agg()
29262935
*-----
29272936
*/
29282937
run_cost = ofpinfo->rel_total_cost - ofpinfo->rel_startup_cost;
2938+
run_cost += outerrel->reltarget->cost.per_tuple * input_rows;
29292939
run_cost += aggcosts.finalCost.per_tuple * numGroups;
29302940
run_cost += cpu_tuple_cost * numGroups;
29312941

src/backend/optimizer/plan/planner.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7113,6 +7113,10 @@ apply_scanjoin_target_to_paths(PlannerInfo *root,
71137113
* confused in createplan.c if they don't agree. We must do this now so
71147114
* that any append paths made in the next part will use the correct
71157115
* pathtarget (cf. create_append_path).
7116+
*
7117+
* Note that this is also necessary if GetForeignUpperPaths() gets called
7118+
* on the final scan/join relation or on any of its children, since the
7119+
* FDW might look at the rel's target to create ForeignPaths.
71167120
*/
71177121
rel->reltarget = llast_node(PathTarget, scanjoin_targets);
71187122

0 commit comments

Comments
 (0)