Fix deparsing of Consts in postgres_fdw ORDER BY
For UNION ALL queries where a union child query contained a foreign table, if the targetlist of that query contained a constant, and the top-level query performed an ORDER BY which contained the column for the constant value, then postgres_fdw would find the EquivalenceMember with the Const and then try to produce an ORDER BY containing that Const. This caused problems with INT typed Consts as these could appear to be requests to order by an ordinal column position rather than the constant value. This could lead to either an error such as: ERROR: ORDER BY position <int const> is not in select list or worse, if the constant value is a valid column, then we could just sort by the wrong column altogether. Here we fix this issue by just not including these Consts in the ORDER BY clause. In passing, add a new section for testing ORDER BY in the postgres_fdw tests and move two existing tests which were misplaced in the WHERE clause testing section into it. Reported-by: Michał Kłeczek Reviewed-by: Ashutosh Bapat, Richard Guo Bug: #18381 Discussion: https://postgr.es/m/0714C8B8-8D82-4ABB-9F8D-A0C3657E7B6E%40kleczek.org Discussion: https://postgr.es/m/18381-137456acd168bf93%40postgresql.org Backpatch-through: 12, oldest supported version
This commit is contained in:
David Rowley
parent
7b8e2ae2fd
commit
c399248b3b
|
|
@ -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);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
-- ===================================================================
|
||||
|
|
|
|||
|
|
@ -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
|
||||
-- ===================================================================
|
||||
|
|
|
|||
Loading…
Reference in New Issue