Skip to content

Commit 332bec1

Browse files
peterepetere
committed
postgres_fdw: Fix join push down with extensions
Objects in an extension are shippable to a foreign server if the extension is part of the foreign server definition's shippable extensions list. But this was not properly considered in some cases when checking whether a join condition can be pushed to a foreign server and the join condition uses an object from a shippable extension. So the join would never be pushed down in those cases. So, the list of extensions needs to 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 report from David Rowley
1 parent 6e033c6 commit 332bec1

File tree

3 files changed

+160
-72
lines changed

3 files changed

+160
-72
lines changed

‎contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1620,6 +1620,35 @@ SELECT t1.c1, t2.c1 FROM ft4 t1 FULL JOIN ft5 t2 ON (t1.c1 = t2.c1) WHERE (t1.c1
16201620
| 21
16211621
(10 rows)
16221622

1623+
-- full outer join + WHERE clause with shippable extensions set
1624+
EXPLAIN (VERBOSE, COSTS OFF)
1625+
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;
1626+
QUERY PLAN
1627+
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
1628+
Limit
1629+
Output: t1.c1, t2.c2, t1.c3
1630+
-> Foreign Scan
1631+
Output: t1.c1, t2.c2, t1.c3
1632+
Relations: (public.ft1 t1) FULL JOIN (public.ft2 t2)
1633+
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))
1634+
(6 rows)
1635+
1636+
ALTER SERVER loopback OPTIONS (DROP extensions);
1637+
-- full outer join + WHERE clause with shippable extensions not set
1638+
EXPLAIN (VERBOSE, COSTS OFF)
1639+
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;
1640+
QUERY PLAN
1641+
-------------------------------------------------------------------------------------------------------------------------------
1642+
Limit
1643+
Output: t1.c1, t2.c2, t1.c3
1644+
-> Foreign Scan
1645+
Output: t1.c1, t2.c2, t1.c3
1646+
Filter: (postgres_fdw_abs(t1.c1) > 0)
1647+
Relations: (public.ft1 t1) FULL JOIN (public.ft2 t2)
1648+
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"))))
1649+
(7 rows)
1650+
1651+
ALTER SERVER loopback OPTIONS (ADD extensions 'postgres_fdw');
16231652
-- join two tables with FOR UPDATE clause
16241653
-- tests whole-row reference for row marks
16251654
EXPLAIN (VERBOSE, COSTS OFF)

‎contrib/postgres_fdw/postgres_fdw.c

Lines changed: 123 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,11 @@ static void add_paths_with_pathkeys_for_rel(PlannerInfo *root, RelOptInfo *rel,
414414
static void add_foreign_grouping_paths(PlannerInfo *root,
415415
RelOptInfo *input_rel,
416416
RelOptInfo *grouped_rel);
417+
static void apply_server_options(PgFdwRelationInfo *fpinfo);
418+
static void apply_table_options(PgFdwRelationInfo *fpinfo);
419+
static void merge_fdw_options(PgFdwRelationInfo *fpinfo,
420+
const PgFdwRelationInfo *fpinfo_o,
421+
const PgFdwRelationInfo *fpinfo_i);
417422

418423

419424
/*
@@ -513,31 +518,8 @@ postgresGetForeignRelSize(PlannerInfo *root,
513518
fpinfo->shippable_extensions = NIL;
514519
fpinfo->fetch_size = 100;
515520

516-
foreach(lc, fpinfo->server->options)
517-
{
518-
DefElem *def = (DefElem *) lfirst(lc);
519-
520-
if (strcmp(def->defname, "use_remote_estimate") == 0)
521-
fpinfo->use_remote_estimate = defGetBoolean(def);
522-
else if (strcmp(def->defname, "fdw_startup_cost") == 0)
523-
fpinfo->fdw_startup_cost = strtod(defGetString(def), NULL);
524-
else if (strcmp(def->defname, "fdw_tuple_cost") == 0)
525-
fpinfo->fdw_tuple_cost = strtod(defGetString(def), NULL);
526-
else if (strcmp(def->defname, "extensions") == 0)
527-
fpinfo->shippable_extensions =
528-
ExtractExtensionList(defGetString(def), false);
529-
else if (strcmp(def->defname, "fetch_size") == 0)
530-
fpinfo->fetch_size = strtol(defGetString(def), NULL, 10);
531-
}
532-
foreach(lc, fpinfo->table->options)
533-
{
534-
DefElem *def = (DefElem *) lfirst(lc);
535-
536-
if (strcmp(def->defname, "use_remote_estimate") == 0)
537-
fpinfo->use_remote_estimate = defGetBoolean(def);
538-
else if (strcmp(def->defname, "fetch_size") == 0)
539-
fpinfo->fetch_size = strtol(defGetString(def), NULL, 10);
540-
}
521+
apply_server_options(fpinfo);
522+
apply_table_options(fpinfo);
541523

542524
/*
543525
* If the table or the server is configured to use remote estimates,
@@ -4114,6 +4096,15 @@ foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype,
41144096
if (fpinfo_o->local_conds || fpinfo_i->local_conds)
41154097
return false;
41164098

4099+
/*
4100+
* Merge FDW options. We might be tempted to do this after we have deemed
4101+
* the foreign join to be OK. But we must do this beforehand so that we
4102+
* know which quals can be evaluated on the foreign server, which might
4103+
* depend on shippable_extensions.
4104+
*/
4105+
fpinfo->server = fpinfo_o->server;
4106+
merge_fdw_options(fpinfo, fpinfo_o, fpinfo_i);
4107+
41174108
/*
41184109
* Separate restrict list into join quals and pushed-down (other) quals.
41194110
*
@@ -4279,15 +4270,6 @@ foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype,
42794270
/* Mark that this join can be pushed down safely */
42804271
fpinfo->pushdown_safe = true;
42814272

4282-
/*
4283-
* If user is willing to estimate cost for a scan of either of the joining
4284-
* relations using EXPLAIN, he intends to estimate scans on that relation
4285-
* more accurately. Then, it makes sense to estimate the cost of the join
4286-
* with that relation more accurately using EXPLAIN.
4287-
*/
4288-
fpinfo->use_remote_estimate = fpinfo_o->use_remote_estimate ||
4289-
fpinfo_i->use_remote_estimate;
4290-
42914273
/* Get user mapping */
42924274
if (fpinfo->use_remote_estimate)
42934275
{
@@ -4299,17 +4281,6 @@ foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype,
42994281
else
43004282
fpinfo->user = NULL;
43014283

4302-
/* Get foreign server */
4303-
fpinfo->server = fpinfo_o->server;
4304-
4305-
/*
4306-
* Since both the joining relations come from the same server, the server
4307-
* level options should have same value for both the relations. Pick from
4308-
* any side.
4309-
*/
4310-
fpinfo->fdw_startup_cost = fpinfo_o->fdw_startup_cost;
4311-
fpinfo->fdw_tuple_cost = fpinfo_o->fdw_tuple_cost;
4312-
43134284
/*
43144285
* Set cached relation costs to some negative value, so that we can detect
43154286
* when they are set to some sensible costs, during one (usually the
@@ -4318,15 +4289,6 @@ foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype,
43184289
fpinfo->rel_startup_cost = -1;
43194290
fpinfo->rel_total_cost = -1;
43204291

4321-
/*
4322-
* Set fetch size to maximum of the joining sides, since we are expecting
4323-
* the rows returned by the join to be proportional to the relation sizes.
4324-
*/
4325-
if (fpinfo_o->fetch_size > fpinfo_i->fetch_size)
4326-
fpinfo->fetch_size = fpinfo_o->fetch_size;
4327-
else
4328-
fpinfo->fetch_size = fpinfo_i->fetch_size;
4329-
43304292
/*
43314293
* Set the string describing this join relation to be used in EXPLAIN
43324294
* output of corresponding ForeignScan.
@@ -4384,6 +4346,110 @@ add_paths_with_pathkeys_for_rel(PlannerInfo *root, RelOptInfo *rel,
43844346
}
43854347
}
43864348

4349+
/*
4350+
* Parse options from foreign server and apply them to fpinfo.
4351+
*
4352+
* New options might also require tweaking merge_fdw_options().
4353+
*/
4354+
static void
4355+
apply_server_options(PgFdwRelationInfo *fpinfo)
4356+
{
4357+
ListCell *lc;
4358+
4359+
foreach(lc, fpinfo->server->options)
4360+
{
4361+
DefElem *def = (DefElem *) lfirst(lc);
4362+
4363+
if (strcmp(def->defname, "use_remote_estimate") == 0)
4364+
fpinfo->use_remote_estimate = defGetBoolean(def);
4365+
else if (strcmp(def->defname, "fdw_startup_cost") == 0)
4366+
fpinfo->fdw_startup_cost = strtod(defGetString(def), NULL);
4367+
else if (strcmp(def->defname, "fdw_tuple_cost") == 0)
4368+
fpinfo->fdw_tuple_cost = strtod(defGetString(def), NULL);
4369+
else if (strcmp(def->defname, "extensions") == 0)
4370+
fpinfo->shippable_extensions =
4371+
ExtractExtensionList(defGetString(def), false);
4372+
else if (strcmp(def->defname, "fetch_size") == 0)
4373+
fpinfo->fetch_size = strtol(defGetString(def), NULL, 10);
4374+
}
4375+
}
4376+
4377+
/*
4378+
* Parse options from foreign table and apply them to fpinfo.
4379+
*
4380+
* New options might also require tweaking merge_fdw_options().
4381+
*/
4382+
static void
4383+
apply_table_options(PgFdwRelationInfo *fpinfo)
4384+
{
4385+
ListCell *lc;
4386+
4387+
foreach(lc, fpinfo->table->options)
4388+
{
4389+
DefElem *def = (DefElem *) lfirst(lc);
4390+
4391+
if (strcmp(def->defname, "use_remote_estimate") == 0)
4392+
fpinfo->use_remote_estimate = defGetBoolean(def);
4393+
else if (strcmp(def->defname, "fetch_size") == 0)
4394+
fpinfo->fetch_size = strtol(defGetString(def), NULL, 10);
4395+
}
4396+
}
4397+
4398+
/*
4399+
* Merge FDW options from input relations into a new set of options for a join
4400+
* or an upper rel.
4401+
*
4402+
* For a join relation, FDW-specific information about the inner and outer
4403+
* relations is provided using fpinfo_i and fpinfo_o. For an upper relation,
4404+
* fpinfo_o provides the information for the input relation; fpinfo_i is
4405+
* expected to NULL.
4406+
*/
4407+
static void
4408+
merge_fdw_options(PgFdwRelationInfo *fpinfo,
4409+
const PgFdwRelationInfo *fpinfo_o,
4410+
const PgFdwRelationInfo *fpinfo_i)
4411+
{
4412+
/* We must always have fpinfo_o. */
4413+
Assert(fpinfo_o);
4414+
4415+
/* fpinfo_i may be NULL, but if present the servers must both match. */
4416+
Assert(!fpinfo_i ||
4417+
fpinfo_i->server->serverid == fpinfo_o->server->serverid);
4418+
4419+
/*
4420+
* Copy the server specific FDW options. (For a join, both relations come
4421+
* from the same server, so the server options should have the same value
4422+
* for both relations.)
4423+
*/
4424+
fpinfo->fdw_startup_cost = fpinfo_o->fdw_startup_cost;
4425+
fpinfo->fdw_tuple_cost = fpinfo_o->fdw_tuple_cost;
4426+
fpinfo->shippable_extensions = fpinfo_o->shippable_extensions;
4427+
fpinfo->use_remote_estimate = fpinfo_o->use_remote_estimate;
4428+
fpinfo->fetch_size = fpinfo_o->fetch_size;
4429+
4430+
/* Merge the table level options from either side of the join. */
4431+
if (fpinfo_i)
4432+
{
4433+
/*
4434+
* We'll prefer to use remote estimates for this join if any table
4435+
* from either side of the join is using remote estimates. This is
4436+
* most likely going to be preferred since they're already willing to
4437+
* pay the price of a round trip to get the remote EXPLAIN. In any
4438+
* case it's not entirely clear how we might otherwise handle this
4439+
* best.
4440+
*/
4441+
fpinfo->use_remote_estimate = fpinfo_o->use_remote_estimate ||
4442+
fpinfo_i->use_remote_estimate;
4443+
4444+
/*
4445+
* Set fetch size to maximum of the joining sides, since we are
4446+
* expecting the rows returned by the join to be proportional to the
4447+
* relation sizes.
4448+
*/
4449+
fpinfo->fetch_size = Max(fpinfo_o->fetch_size, fpinfo_i->fetch_size);
4450+
}
4451+
}
4452+
43874453
/*
43884454
* postgresGetForeignJoinPaths
43894455
* Add possible ForeignPath to joinrel, if join is safe to push down.
@@ -4714,18 +4780,6 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel)
47144780
/* Safe to pushdown */
47154781
fpinfo->pushdown_safe = true;
47164782

4717-
/*
4718-
* If user is willing to estimate cost for a scan using EXPLAIN, he
4719-
* intends to estimate scans on that relation more accurately. Then, it
4720-
* makes sense to estimate the cost of the grouping on that relation more
4721-
* accurately using EXPLAIN.
4722-
*/
4723-
fpinfo->use_remote_estimate = ofpinfo->use_remote_estimate;
4724-
4725-
/* Copy startup and tuple cost as is from underneath input rel's fpinfo */
4726-
fpinfo->fdw_startup_cost = ofpinfo->fdw_startup_cost;
4727-
fpinfo->fdw_tuple_cost = ofpinfo->fdw_tuple_cost;
4728-
47294783
/*
47304784
* Set cached relation costs to some negative value, so that we can detect
47314785
* when they are set to some sensible costs, during one (usually the
@@ -4734,9 +4788,6 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel)
47344788
fpinfo->rel_startup_cost = -1;
47354789
fpinfo->rel_total_cost = -1;
47364790

4737-
/* Set fetch size same as that of underneath input rel's fpinfo */
4738-
fpinfo->fetch_size = ofpinfo->fetch_size;
4739-
47404791
/*
47414792
* Set the string describing this grouped relation to be used in EXPLAIN
47424793
* output of corresponding ForeignScan.
@@ -4812,13 +4863,13 @@ add_foreign_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
48124863
fpinfo->outerrel = input_rel;
48134864

48144865
/*
4815-
* Copy foreign table, foreign server, user mapping, shippable extensions
4816-
* etc. details from the input relation's fpinfo.
4866+
* Copy foreign table, foreign server, user mapping, FDW options etc.
4867+
* details from the input relation's fpinfo.
48174868
*/
48184869
fpinfo->table = ifpinfo->table;
48194870
fpinfo->server = ifpinfo->server;
48204871
fpinfo->user = ifpinfo->user;
4821-
fpinfo->shippable_extensions = ifpinfo->shippable_extensions;
4872+
merge_fdw_options(fpinfo, ifpinfo , NULL);
48224873

48234874
/* Assess if it is safe to push down aggregation and grouping. */
48244875
if (!foreign_grouping_ok(root, grouped_rel))

‎contrib/postgres_fdw/sql/postgres_fdw.sql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,14 @@ SELECT t1.c1, t2.c2, t3.c3 FROM ft2 t1 LEFT JOIN ft2 t2 ON (t1.c1 = t2.c1) RIGHT
447447
EXPLAIN (VERBOSE, COSTS OFF)
448448
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;
449449
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;
450+
-- full outer join + WHERE clause with shippable extensions set
451+
EXPLAIN (VERBOSE, COSTS OFF)
452+
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;
453+
ALTER SERVER loopback OPTIONS (DROP extensions);
454+
-- full outer join + WHERE clause with shippable extensions not set
455+
EXPLAIN (VERBOSE, COSTS OFF)
456+
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;
457+
ALTER SERVER loopback OPTIONS (ADD extensions 'postgres_fdw');
450458
-- join two tables with FOR UPDATE clause
451459
-- tests whole-row reference for row marks
452460
EXPLAIN (VERBOSE, COSTS OFF)

0 commit comments

Comments
 (0)