On 2016/06/08 23:16, Amit Langote wrote: > On Wed, Jun 8, 2016 at 9:27 PM, Robert Haas <robertmh...@gmail.com> wrote: >> On Tue, Jun 7, 2016 at 10:41 PM, Noah Misch <n...@leadboat.com> wrote: >>> [Action required within 72 hours. This is a generic notification.] >>> >>> The above-described topic is currently a PostgreSQL 9.6 open item. Robert, >>> since you committed the patch believed to have created it, you own this open >>> item. If some other commit is more relevant or if this does not belong as a >>> 9.6 open item, please let us know. Otherwise, please observe the policy on >>> open item ownership[1] and send a status update within 72 hours of this >>> message. Include a date for your subsequent status update. Testers may >>> discover new open items at any time, and I want to plan to get them all >>> fixed >>> well in advance of shipping 9.6rc1. Consequently, I will appreciate your >>> efforts toward speedy resolution. Thanks. >> >> Discussion of this issue is still ongoing. Accordingly, I intend to >> wait until that discussion has concluded before proceeding further. >> I'll check this thread again no later than Friday and send an update >> by then. > > Ashutosh seemed OK with the latest patch.
I adjusted some comments per off-list suggestion from Ashutosh. Please find attached the new version. Thanks, Amit
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index 1de0bc4..0468095 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -2202,6 +2202,36 @@ SELECT t1.c1, t2.c1 FROM (ft5 t1 JOIN v_ft5 t2 ON (t1.c1 = t2.c1)) left join (ft Remote SQL: SELECT c1 FROM "S 1"."T 4" (27 rows) +-- query which introduces placeholders in the targetlist cannot be +-- pushed down +EXPLAIN (COSTS false, VERBOSE) +SELECT q.a, ft2.c1 FROM (SELECT 13 as a FROM ft1 WHERE c1 = 13) q RIGHT JOIN ft2 ON (q.a = ft2.c1) WHERE ft2.c1 BETWEEN 10 AND 15; + QUERY PLAN +--------------------------------------------------------------------------------------------------------------------------- + Nested Loop Left Join + Output: (13), ft2.c1 + Join Filter: (13 = ft2.c1) + -> Foreign Scan on public.ft2 + Output: ft2.c1 + Remote SQL: SELECT "C 1" FROM "S 1"."T 1" WHERE (("C 1" >= 10)) AND (("C 1" <= 15)) ORDER BY "C 1" ASC NULLS LAST + -> Materialize + Output: (13) + -> Foreign Scan on public.ft1 + Output: 13 + Remote SQL: SELECT NULL FROM "S 1"."T 1" WHERE (("C 1" = 13)) +(11 rows) + +SELECT q.a, ft2.c1 FROM (SELECT 13 as a FROM ft1 WHERE c1 = 13) q RIGHT JOIN ft2 ON (q.a = ft2.c1) WHERE ft2.c1 BETWEEN 10 AND 15; + a | c1 +----+---- + | 10 + | 11 + | 12 + 13 | 13 + | 14 + | 15 +(6 rows) + -- recreate the dropped user mapping for further tests CREATE USER MAPPING FOR CURRENT_USER SERVER loopback; DROP USER MAPPING FOR PUBLIC SERVER loopback; diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 4d17272..f010124 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -3960,6 +3960,8 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid) * 3) All join conditions are safe to push down * 4) No relation has local filter (this can be relaxed for INNER JOIN, if we * can move unpushable clauses upwards in the join tree). + * 5) There is no PlaceHolderVar originating at the joinrel that is being + * referenced elsewhere. */ static bool foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype, @@ -4036,6 +4038,25 @@ foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype, return false; } + /* + * Cannot push down the join if any PlaceHolderVar in its targetlist is + * needed elsewhere. That is so because the correct result cannot be + * ensured from the foreign join query with deparse code currently being + * unable to generate and push down subqueries. Remember that such a + * PHV was created when some subquery on nullable side of an outer join, + * with a non-nullable expression in its targetlist (wrapped in the PHV), + * was pulled up during the planner prep phase. + */ + foreach(lc, root->placeholder_list) + { + PlaceHolderInfo *phinfo = lfirst(lc); + Relids relids = joinrel->relids; + + if (bms_nonempty_difference(phinfo->ph_needed, relids) && + bms_is_subset(phinfo->ph_eval_at, relids)) + return false; + } + /* Save the join clauses, for later use. */ fpinfo->joinclauses = joinclauses; diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 6c2b08c..5ae3d53 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -527,6 +527,12 @@ EXECUTE join_stmt; EXPLAIN (COSTS false, VERBOSE) SELECT t1.c1, t2.c1 FROM (ft5 t1 JOIN v_ft5 t2 ON (t1.c1 = t2.c1)) left join (ft5 t3 JOIN v_ft5 t4 ON (t3.c1 = t4.c1)) ON (t1.c1 = t3.c1); +-- query which introduces placeholders in the targetlist cannot be +-- pushed down +EXPLAIN (COSTS false, VERBOSE) +SELECT q.a, ft2.c1 FROM (SELECT 13 as a FROM ft1 WHERE c1 = 13) q RIGHT JOIN ft2 ON (q.a = ft2.c1) WHERE ft2.c1 BETWEEN 10 AND 15; +SELECT q.a, ft2.c1 FROM (SELECT 13 as a FROM ft1 WHERE c1 = 13) q RIGHT JOIN ft2 ON (q.a = ft2.c1) WHERE ft2.c1 BETWEEN 10 AND 15; + -- recreate the dropped user mapping for further tests CREATE USER MAPPING FOR CURRENT_USER SERVER loopback; DROP USER MAPPING FOR PUBLIC SERVER loopback;
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers