diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c index 8fc66fa11c..8787c00d3e 100644 --- a/contrib/postgres_fdw/deparse.c +++ b/contrib/postgres_fdw/deparse.c @@ -3912,13 +3912,12 @@ appendOrderByClause(List *pathkeys, bool has_final_sort, { ListCell *lcell; int nestlevel; - const char *delim = " "; StringInfo buf = context->buf; + bool gotone = false; /* Make sure any constants in the exprs are printed portably */ nestlevel = set_transmission_modes(); - appendStringInfoString(buf, " ORDER BY"); foreach(lcell, pathkeys) { PathKey *pathkey = lfirst(lcell); @@ -3951,6 +3950,26 @@ appendOrderByClause(List *pathkeys, bool has_final_sort, em_expr = em->em_expr; + /* + * If the member is a Const expression then we needn't add it to the + * ORDER BY clause. This can happen in UNION ALL queries where the + * union child targetlist has a Const. Adding these would be + * wasteful, but also, for INT columns, an integer literal would be + * seen as an ordinal column position rather than a value to sort by. + * deparseConst() does have code to handle this, but it seems less + * effort on all accounts just to skip these for ORDER BY clauses. + */ + if (IsA(em_expr, Const)) + continue; + + if (!gotone) + { + appendStringInfoString(buf, " ORDER BY "); + gotone = true; + } + else + appendStringInfoString(buf, ", "); + /* * Lookup the operator corresponding to the strategy in the opclass. * The datatype used by the opfamily is not necessarily the same as @@ -3965,7 +3984,6 @@ appendOrderByClause(List *pathkeys, bool has_final_sort, pathkey->pk_strategy, em->em_datatype, em->em_datatype, pathkey->pk_opfamily); - appendStringInfoString(buf, delim); deparseExpr(em_expr, context); /* @@ -3975,7 +3993,6 @@ appendOrderByClause(List *pathkeys, bool has_final_sort, appendOrderBySuffix(oprid, exprType((Node *) em_expr), pathkey->pk_nulls_first, context); - delim = ", "; } reset_transmission_modes(nestlevel); } diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index c355e8f3f7..58a603ac56 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -893,32 +893,6 @@ SELECT * FROM ft2 WHERE c1 = ANY (ARRAY(SELECT c1 FROM ft1 WHERE c1 < 5)); 4 | 4 | 00004 | Mon Jan 05 00:00:00 1970 PST | Mon Jan 05 00:00:00 1970 | 4 | 4 | foo (4 rows) --- we should not push order by clause with volatile expressions or unsafe --- collations -EXPLAIN (VERBOSE, COSTS OFF) - SELECT * FROM ft2 ORDER BY ft2.c1, random(); - QUERY PLAN -------------------------------------------------------------------------------- - Sort - Output: c1, c2, c3, c4, c5, c6, c7, c8, (random()) - Sort Key: ft2.c1, (random()) - -> Foreign Scan on public.ft2 - Output: c1, c2, c3, c4, c5, c6, c7, c8, random() - Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" -(6 rows) - -EXPLAIN (VERBOSE, COSTS OFF) - SELECT * FROM ft2 ORDER BY ft2.c1, ft2.c3 collate "C"; - QUERY PLAN -------------------------------------------------------------------------------- - Sort - Output: c1, c2, c3, c4, c5, c6, c7, c8, ((c3)::text) - Sort Key: ft2.c1, ft2.c3 COLLATE "C" - -> Foreign Scan on public.ft2 - Output: c1, c2, c3, c4, c5, c6, c7, c8, c3 - Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" -(6 rows) - -- user-defined operator/function CREATE FUNCTION postgres_fdw_abs(int) RETURNS int AS $$ BEGIN @@ -1206,6 +1180,73 @@ WHERE c1 = 642 AND length(to_tsvector('custom_search'::regconfig, c3)) > 0; 642 | '00642':1 (1 row) +-- =================================================================== +-- ORDER BY queries +-- =================================================================== +-- we should not push order by clause with volatile expressions or unsafe +-- collations +EXPLAIN (VERBOSE, COSTS OFF) + SELECT * FROM ft2 ORDER BY ft2.c1, random(); + QUERY PLAN +------------------------------------------------------------------------------- + Sort + Output: c1, c2, c3, c4, c5, c6, c7, c8, (random()) + Sort Key: ft2.c1, (random()) + -> Foreign Scan on public.ft2 + Output: c1, c2, c3, c4, c5, c6, c7, c8, random() + Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" +(6 rows) + +EXPLAIN (VERBOSE, COSTS OFF) + SELECT * FROM ft2 ORDER BY ft2.c1, ft2.c3 collate "C"; + QUERY PLAN +------------------------------------------------------------------------------- + Sort + Output: c1, c2, c3, c4, c5, c6, c7, c8, ((c3)::text) + Sort Key: ft2.c1, ft2.c3 COLLATE "C" + -> Foreign Scan on public.ft2 + Output: c1, c2, c3, c4, c5, c6, c7, c8, c3 + Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" +(6 rows) + +-- Ensure we don't push ORDER BY expressions which are Consts at the UNION +-- child level to the foreign server. +EXPLAIN (VERBOSE, COSTS OFF) +SELECT * FROM ( + SELECT 1 AS type,c1 FROM ft1 + UNION ALL + SELECT 2 AS type,c1 FROM ft2 +) a ORDER BY type,c1; + QUERY PLAN +--------------------------------------------------------------------------------- + Merge Append + Sort Key: (1), ft1.c1 + -> Foreign Scan on public.ft1 + Output: 1, ft1.c1 + Remote SQL: SELECT "C 1" FROM "S 1"."T 1" ORDER BY "C 1" ASC NULLS LAST + -> Foreign Scan on public.ft2 + Output: 2, ft2.c1 + Remote SQL: SELECT "C 1" FROM "S 1"."T 1" ORDER BY "C 1" ASC NULLS LAST +(8 rows) + +EXPLAIN (VERBOSE, COSTS OFF) +SELECT * FROM ( + SELECT 1 AS type,c1 FROM ft1 + UNION ALL + SELECT 2 AS type,c1 FROM ft2 +) a ORDER BY type; + QUERY PLAN +--------------------------------------------------- + Merge Append + Sort Key: (1) + -> Foreign Scan on public.ft1 + Output: 1, ft1.c1 + Remote SQL: SELECT "C 1" FROM "S 1"."T 1" + -> Foreign Scan on public.ft2 + Output: 2, ft2.c1 + Remote SQL: SELECT "C 1" FROM "S 1"."T 1" +(8 rows) + -- =================================================================== -- JOIN queries -- =================================================================== diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 812e7646e1..e3d147de6d 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -355,12 +355,6 @@ WHERE a.c2 = 6 AND b.c1 = a.c1 AND a.c8 = 'foo' AND b.c7 = upper(a.c7); -- bug before 9.3.5 due to sloppy handling of remote-estimate parameters SELECT * FROM ft1 WHERE c1 = ANY (ARRAY(SELECT c1 FROM ft2 WHERE c1 < 5)); SELECT * FROM ft2 WHERE c1 = ANY (ARRAY(SELECT c1 FROM ft1 WHERE c1 < 5)); --- we should not push order by clause with volatile expressions or unsafe --- collations -EXPLAIN (VERBOSE, COSTS OFF) - SELECT * FROM ft2 ORDER BY ft2.c1, random(); -EXPLAIN (VERBOSE, COSTS OFF) - SELECT * FROM ft2 ORDER BY ft2.c1, ft2.c3 collate "C"; -- user-defined operator/function CREATE FUNCTION postgres_fdw_abs(int) RETURNS int AS $$ @@ -462,6 +456,32 @@ WHERE c1 = 642 AND length(to_tsvector('custom_search'::regconfig, c3)) > 0; SELECT c1, to_tsvector('custom_search'::regconfig, c3) FROM ft1 WHERE c1 = 642 AND length(to_tsvector('custom_search'::regconfig, c3)) > 0; +-- =================================================================== +-- ORDER BY queries +-- =================================================================== +-- we should not push order by clause with volatile expressions or unsafe +-- collations +EXPLAIN (VERBOSE, COSTS OFF) + SELECT * FROM ft2 ORDER BY ft2.c1, random(); +EXPLAIN (VERBOSE, COSTS OFF) + SELECT * FROM ft2 ORDER BY ft2.c1, ft2.c3 collate "C"; + +-- Ensure we don't push ORDER BY expressions which are Consts at the UNION +-- child level to the foreign server. +EXPLAIN (VERBOSE, COSTS OFF) +SELECT * FROM ( + SELECT 1 AS type,c1 FROM ft1 + UNION ALL + SELECT 2 AS type,c1 FROM ft2 +) a ORDER BY type,c1; + +EXPLAIN (VERBOSE, COSTS OFF) +SELECT * FROM ( + SELECT 1 AS type,c1 FROM ft1 + UNION ALL + SELECT 2 AS type,c1 FROM ft2 +) a ORDER BY type; + -- =================================================================== -- JOIN queries -- ===================================================================