From 88d5dd279f7da5df6056c3d341e58aa4a95ce24f Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat Date: Wed, 12 Apr 2017 14:59:50 +0530 Subject: [PATCH] Set shippable extensions for a join relation being pushed down. Objects in an extension are shippable to a foreign server if the extension is part of the server's shippable extensions list. So, the list of extensions should be made available in fpinfo of the relation being considered to be pushed down before any expressions are assessed for being shippable. Fix foreign_join_ok() to do that for a join relation. The code to save FDW options in fpinfo is scattered at multiple places. Bring all of that together into functions apply_server_options(), apply_table_options() and merge_fdw_options(). David Rowley and Ashutosh Bapat, per gripe from David Rowley. --- contrib/postgres_fdw/expected/postgres_fdw.out | 29 ++++ contrib/postgres_fdw/postgres_fdw.c | 194 +++++++++++++++--------- contrib/postgres_fdw/sql/postgres_fdw.sql | 8 + 3 files changed, 160 insertions(+), 71 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index b29549a..c5c34af 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -1620,6 +1620,35 @@ SELECT t1.c1, t2.c1 FROM ft4 t1 FULL JOIN ft5 t2 ON (t1.c1 = t2.c1) WHERE (t1.c1 | 21 (10 rows) +-- full outer join + WHERE clause with shippable extension set +EXPLAIN (VERBOSE, COSTS OFF) +SELECT t1.c1, t2.c2, t1.c3 FROM ft1 t1 FULL JOIN ft2 t2 ON (t1.c1 = t2.c1) WHERE postgres_fdw_abs(t1.c1) > 0 OFFSET 10 LIMIT 10; + QUERY PLAN +------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + Limit + Output: t1.c1, t2.c2, t1.c3 + -> Foreign Scan + Output: t1.c1, t2.c2, t1.c3 + Relations: (public.ft1 t1) FULL JOIN (public.ft2 t2) + Remote SQL: SELECT r1."C 1", r1.c3, r2.c2 FROM ("S 1"."T 1" r1 FULL JOIN "S 1"."T 1" r2 ON (((r1."C 1" = r2."C 1")))) WHERE ((public.postgres_fdw_abs(r1."C 1") > 0)) +(6 rows) + +ALTER SERVER loopback OPTIONS (DROP extensions); +-- full outer join + WHERE clause with shippable extension not set +EXPLAIN (VERBOSE, COSTS OFF) +SELECT t1.c1, t2.c2, t1.c3 FROM ft1 t1 FULL JOIN ft2 t2 ON (t1.c1 = t2.c1) WHERE postgres_fdw_abs(t1.c1) > 0 OFFSET 10 LIMIT 10; + QUERY PLAN +------------------------------------------------------------------------------------------------------------------------------- + Limit + Output: t1.c1, t2.c2, t1.c3 + -> Foreign Scan + Output: t1.c1, t2.c2, t1.c3 + Filter: (postgres_fdw_abs(t1.c1) > 0) + Relations: (public.ft1 t1) FULL JOIN (public.ft2 t2) + Remote SQL: SELECT r1."C 1", r1.c3, r2.c2 FROM ("S 1"."T 1" r1 FULL JOIN "S 1"."T 1" r2 ON (((r1."C 1" = r2."C 1")))) +(7 rows) + +ALTER SERVER loopback OPTIONS (ADD extensions 'postgres_fdw'); -- join two tables with FOR UPDATE clause -- tests whole-row reference for row marks EXPLAIN (VERBOSE, COSTS OFF) diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 8d02243..2ff3ad3 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -414,6 +414,11 @@ 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); +static void apply_server_options(PgFdwRelationInfo *fpinfo); +static void apply_table_options(PgFdwRelationInfo *fpinfo); +static void merge_fdw_options(PgFdwRelationInfo *fpinfo, + PgFdwRelationInfo *fpinfo_o, + PgFdwRelationInfo *fpinfo_i); /* @@ -513,31 +518,9 @@ postgresGetForeignRelSize(PlannerInfo *root, fpinfo->shippable_extensions = NIL; fpinfo->fetch_size = 100; - foreach(lc, fpinfo->server->options) - { - DefElem *def = (DefElem *) lfirst(lc); + apply_server_options(fpinfo); - if (strcmp(def->defname, "use_remote_estimate") == 0) - fpinfo->use_remote_estimate = defGetBoolean(def); - else if (strcmp(def->defname, "fdw_startup_cost") == 0) - fpinfo->fdw_startup_cost = strtod(defGetString(def), NULL); - else if (strcmp(def->defname, "fdw_tuple_cost") == 0) - fpinfo->fdw_tuple_cost = strtod(defGetString(def), NULL); - else if (strcmp(def->defname, "extensions") == 0) - fpinfo->shippable_extensions = - ExtractExtensionList(defGetString(def), false); - else if (strcmp(def->defname, "fetch_size") == 0) - fpinfo->fetch_size = strtol(defGetString(def), NULL, 10); - } - foreach(lc, fpinfo->table->options) - { - DefElem *def = (DefElem *) lfirst(lc); - - if (strcmp(def->defname, "use_remote_estimate") == 0) - fpinfo->use_remote_estimate = defGetBoolean(def); - else if (strcmp(def->defname, "fetch_size") == 0) - fpinfo->fetch_size = strtol(defGetString(def), NULL, 10); - } + apply_table_options(fpinfo); /* * If the table or the server is configured to use remote estimates, @@ -4115,6 +4098,15 @@ foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype, return false; /* + * This function should usually set FDW options in fpinfo after the join is + * deemed safe to push down to save some CPU cycles. But We need server + * specific options like extensions to decide push-down safety. For + * checking extension shippability, we need foreign server as well. + */ + fpinfo->server = fpinfo_o->server; + merge_fdw_options(fpinfo, fpinfo_o, fpinfo_i); + + /* * Separate restrict list into join quals and pushed-down (other) quals. * * Join quals belonging to an outer join must all be shippable, else we @@ -4279,15 +4271,6 @@ foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype, /* Mark that this join can be pushed down safely */ fpinfo->pushdown_safe = true; - /* - * If user is willing to estimate cost for a scan of either of the joining - * relations using EXPLAIN, he intends to estimate scans on that relation - * more accurately. Then, it makes sense to estimate the cost of the join - * with that relation more accurately using EXPLAIN. - */ - fpinfo->use_remote_estimate = fpinfo_o->use_remote_estimate || - fpinfo_i->use_remote_estimate; - /* Get user mapping */ if (fpinfo->use_remote_estimate) { @@ -4299,17 +4282,6 @@ foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype, else fpinfo->user = NULL; - /* Get foreign server */ - fpinfo->server = fpinfo_o->server; - - /* - * Since both the joining relations come from the same server, the server - * level options should have same value for both the relations. Pick from - * any side. - */ - fpinfo->fdw_startup_cost = fpinfo_o->fdw_startup_cost; - fpinfo->fdw_tuple_cost = fpinfo_o->fdw_tuple_cost; - /* * Set cached relation costs to some negative value, so that we can detect * when they are set to some sensible costs, during one (usually the @@ -4319,15 +4291,6 @@ foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype, fpinfo->rel_total_cost = -1; /* - * Set fetch size to maximum of the joining sides, since we are expecting - * the rows returned by the join to be proportional to the relation sizes. - */ - if (fpinfo_o->fetch_size > fpinfo_i->fetch_size) - fpinfo->fetch_size = fpinfo_o->fetch_size; - else - fpinfo->fetch_size = fpinfo_i->fetch_size; - - /* * Set the string describing this join relation to be used in EXPLAIN * output of corresponding ForeignScan. */ @@ -4385,6 +4348,110 @@ add_paths_with_pathkeys_for_rel(PlannerInfo *root, RelOptInfo *rel, } /* + * apply_server_options + * Parse options from foreign server any apply them to 'fpinfo' + * + * New options may also require tweaking merge_fdw_options() + */ +static void +apply_server_options(PgFdwRelationInfo *fpinfo) +{ + ListCell *lc; + + foreach(lc, fpinfo->server->options) + { + DefElem *def = (DefElem *) lfirst(lc); + + if (strcmp(def->defname, "use_remote_estimate") == 0) + fpinfo->use_remote_estimate = defGetBoolean(def); + else if (strcmp(def->defname, "fdw_startup_cost") == 0) + fpinfo->fdw_startup_cost = strtod(defGetString(def), NULL); + else if (strcmp(def->defname, "fdw_tuple_cost") == 0) + fpinfo->fdw_tuple_cost = strtod(defGetString(def), NULL); + else if (strcmp(def->defname, "extensions") == 0) + fpinfo->shippable_extensions = + ExtractExtensionList(defGetString(def), false); + else if (strcmp(def->defname, "fetch_size") == 0) + fpinfo->fetch_size = strtol(defGetString(def), NULL, 10); + } +} + +/* + * apply_table_options + * Parse options from foreign table any apply them to 'fpinfo' + * + * New options may also require tweaking merge_fdw_options() + */ +static void +apply_table_options(PgFdwRelationInfo *fpinfo) +{ + ListCell *lc; + + foreach(lc, fpinfo->table->options) + { + DefElem *def = (DefElem *) lfirst(lc); + + if (strcmp(def->defname, "use_remote_estimate") == 0) + fpinfo->use_remote_estimate = defGetBoolean(def); + else if (strcmp(def->defname, "fetch_size") == 0) + fpinfo->fetch_size = strtol(defGetString(def), NULL, 10); + } +} + +/* + * merge_fdw_options + * Merge FDW options from input relations into a new set of options for a + * join or an upper rel. + + * For a join relation FDW specific information about both the inner and outer + * relations is provided using fpinfo_i and fpinfo_o resp. For an upper + * relation fpinfo_o provides the same for the input relation; fpinfo_i is + * expected to NULL. With a join rel, the foreign server on either side of the + * join is expected to match, this means we can derive server options from + * either side, although we choose outer. Some table level options may require + * merging. + */ +static void +merge_fdw_options(PgFdwRelationInfo *fpinfo, PgFdwRelationInfo *fpinfo_o, + PgFdwRelationInfo *fpinfo_i) +{ + /* We must always have fpinfo_o. */ + Assert(fpinfo_o); + + /* fpinfo_i may be NULL, but if present the servers must both match */ + Assert(!fpinfo_i || + fpinfo_i->server->serverid == fpinfo_o->server->serverid); + + /* Copy the server specific FDW options from the outer relation. */ + fpinfo->fdw_startup_cost = fpinfo_o->fdw_startup_cost; + fpinfo->fdw_tuple_cost = fpinfo_o->fdw_tuple_cost; + fpinfo->shippable_extensions = fpinfo_o->shippable_extensions; + fpinfo->use_remote_estimate = fpinfo_o->use_remote_estimate; + fpinfo->fetch_size = fpinfo_o->fetch_size; + + /* Merge the table level options from either side of the join. */ + if (fpinfo_i) + { + /* + * We'll prefer to use remote estimates for this join if any table + * from either side of the join is using remote estimates. This is + * most likely going to be preferred since they're already willing to + * pay the price of a round trip to get the remote EXPLAIN. In any + * case it's not entirely clear how best else we might handle this. + */ + fpinfo->use_remote_estimate = fpinfo_o->use_remote_estimate || + fpinfo_i->use_remote_estimate; + + /* + * Set fetch size to maximum of the joining sides, since we are + * expecting the rows returned by the join to be proportional to the + * relation sizes. + */ + fpinfo->fetch_size = Max(fpinfo_o->fetch_size, fpinfo_i->fetch_size); + } +} + +/* * postgresGetForeignJoinPaths * Add possible ForeignPath to joinrel, if join is safe to push down. */ @@ -4715,18 +4782,6 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel) fpinfo->pushdown_safe = true; /* - * If user is willing to estimate cost for a scan using EXPLAIN, he - * intends to estimate scans on that relation more accurately. Then, it - * makes sense to estimate the cost of the grouping on that relation more - * accurately using EXPLAIN. - */ - fpinfo->use_remote_estimate = ofpinfo->use_remote_estimate; - - /* Copy startup and tuple cost as is from underneath input rel's fpinfo */ - fpinfo->fdw_startup_cost = ofpinfo->fdw_startup_cost; - fpinfo->fdw_tuple_cost = ofpinfo->fdw_tuple_cost; - - /* * Set cached relation costs to some negative value, so that we can detect * when they are set to some sensible costs, during one (usually the * first) of the calls to estimate_path_cost_size(). @@ -4734,9 +4789,6 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel) fpinfo->rel_startup_cost = -1; fpinfo->rel_total_cost = -1; - /* Set fetch size same as that of underneath input rel's fpinfo */ - fpinfo->fetch_size = ofpinfo->fetch_size; - /* * Set the string describing this grouped relation to be used in EXPLAIN * output of corresponding ForeignScan. @@ -4812,13 +4864,13 @@ add_foreign_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel, fpinfo->outerrel = input_rel; /* - * Copy foreign table, foreign server, user mapping, shippable extensions - * etc. details from the input relation's fpinfo. + * Copy foreign table, foreign server, user mapping, FDW options etc. + * details from the input relation's fpinfo. */ fpinfo->table = ifpinfo->table; fpinfo->server = ifpinfo->server; fpinfo->user = ifpinfo->user; - fpinfo->shippable_extensions = ifpinfo->shippable_extensions; + merge_fdw_options(fpinfo, ifpinfo , NULL); /* Assess if it is safe to push down aggregation and grouping. */ if (!foreign_grouping_ok(root, grouped_rel)) diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 423eb02..9b8c21e 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -447,6 +447,14 @@ SELECT t1.c1, t2.c2, t3.c3 FROM ft2 t1 LEFT JOIN ft2 t2 ON (t1.c1 = t2.c1) RIGHT EXPLAIN (VERBOSE, COSTS OFF) SELECT t1.c1, t2.c1 FROM ft4 t1 FULL JOIN ft5 t2 ON (t1.c1 = t2.c1) WHERE (t1.c1 = t2.c1 OR t1.c1 IS NULL) ORDER BY t1.c1, t2.c1 OFFSET 10 LIMIT 10; SELECT t1.c1, t2.c1 FROM ft4 t1 FULL JOIN ft5 t2 ON (t1.c1 = t2.c1) WHERE (t1.c1 = t2.c1 OR t1.c1 IS NULL) ORDER BY t1.c1, t2.c1 OFFSET 10 LIMIT 10; +-- full outer join + WHERE clause with shippable extension set +EXPLAIN (VERBOSE, COSTS OFF) +SELECT t1.c1, t2.c2, t1.c3 FROM ft1 t1 FULL JOIN ft2 t2 ON (t1.c1 = t2.c1) WHERE postgres_fdw_abs(t1.c1) > 0 OFFSET 10 LIMIT 10; +ALTER SERVER loopback OPTIONS (DROP extensions); +-- full outer join + WHERE clause with shippable extension not set +EXPLAIN (VERBOSE, COSTS OFF) +SELECT t1.c1, t2.c2, t1.c3 FROM ft1 t1 FULL JOIN ft2 t2 ON (t1.c1 = t2.c1) WHERE postgres_fdw_abs(t1.c1) > 0 OFFSET 10 LIMIT 10; +ALTER SERVER loopback OPTIONS (ADD extensions 'postgres_fdw'); -- join two tables with FOR UPDATE clause -- tests whole-row reference for row marks EXPLAIN (VERBOSE, COSTS OFF) -- 1.7.9.5