Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 48d9d8e

Browse files
committed
Fix a couple of planner bugs introduced by the new ability to discard
ORDER BY <constant> as redundant. One is that this means query_planner() has to canonicalize pathkeys even when the query jointree is empty; the canonicalization was always a no-op in such cases before, but no more. Also, we have to guard against thinking that a set-returning function is "constant" for this purpose. Add a couple of regression tests for these evidently under-tested cases. Per report from Greg Stark and subsequent experimentation.
1 parent d5eaa63 commit 48d9d8e

File tree

4 files changed

+70
-8
lines changed

4 files changed

+70
-8
lines changed

src/backend/optimizer/path/equivclass.c

+9-7
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* Portions Copyright (c) 1994, Regents of the University of California
1111
*
1212
* IDENTIFICATION
13-
* $PostgreSQL: pgsql/src/backend/optimizer/path/equivclass.c,v 1.2 2007/01/22 20:00:39 tgl Exp $
13+
* $PostgreSQL: pgsql/src/backend/optimizer/path/equivclass.c,v 1.3 2007/07/07 20:46:45 tgl Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
@@ -328,8 +328,8 @@ add_eq_member(EquivalenceClass *ec, Expr *expr, Relids relids,
328328
/*
329329
* No Vars, assume it's a pseudoconstant. This is correct for
330330
* entries generated from process_equivalence(), because a WHERE
331-
* clause can't contain aggregates and non-volatility was checked
332-
* before process_equivalence() ever got called. But
331+
* clause can't contain aggregates or SRFs, and non-volatility was
332+
* checked before process_equivalence() ever got called. But
333333
* get_eclass_for_sort_expr() has to work harder. We put the tests
334334
* there not here to save cycles in the equivalence case.
335335
*/
@@ -428,13 +428,15 @@ get_eclass_for_sort_expr(PlannerInfo *root,
428428
false, expr_datatype);
429429

430430
/*
431-
* add_eq_member doesn't check for volatile functions or aggregates,
432-
* but such could appear in sort expressions, so we have to check
433-
* whether its const-marking was correct.
431+
* add_eq_member doesn't check for volatile functions, set-returning
432+
* functions, or aggregates, but such could appear in sort expressions;
433+
* so we have to check whether its const-marking was correct.
434434
*/
435435
if (newec->ec_has_const)
436436
{
437-
if (newec->ec_has_volatile || contain_agg_clause((Node *) expr))
437+
if (newec->ec_has_volatile ||
438+
expression_returns_set((Node *) expr) ||
439+
contain_agg_clause((Node *) expr))
438440
{
439441
newec->ec_has_const = false;
440442
newem->em_is_const = false;

src/backend/optimizer/plan/planmain.c

+13-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
*
1515
*
1616
* IDENTIFICATION
17-
* $PostgreSQL: pgsql/src/backend/optimizer/plan/planmain.c,v 1.101 2007/05/04 01:13:44 tgl Exp $
17+
* $PostgreSQL: pgsql/src/backend/optimizer/plan/planmain.c,v 1.102 2007/07/07 20:46:45 tgl Exp $
1818
*
1919
*-------------------------------------------------------------------------
2020
*/
@@ -106,9 +106,21 @@ query_planner(PlannerInfo *root, List *tlist,
106106
*/
107107
if (parse->jointree->fromlist == NIL)
108108
{
109+
/* We need a trivial path result */
109110
*cheapest_path = (Path *)
110111
create_result_path((List *) parse->jointree->quals);
111112
*sorted_path = NULL;
113+
/*
114+
* We still are required to canonicalize any pathkeys, in case
115+
* it's something like "SELECT 2+2 ORDER BY 1".
116+
*/
117+
root->canon_pathkeys = NIL;
118+
root->query_pathkeys = canonicalize_pathkeys(root,
119+
root->query_pathkeys);
120+
root->group_pathkeys = canonicalize_pathkeys(root,
121+
root->group_pathkeys);
122+
root->sort_pathkeys = canonicalize_pathkeys(root,
123+
root->sort_pathkeys);
112124
return;
113125
}
114126

src/test/regress/expected/select.out

+32
Original file line numberDiff line numberDiff line change
@@ -736,3 +736,35 @@ SELECT * FROM foo ORDER BY f1 DESC NULLS LAST;
736736

737737
(7 rows)
738738

739+
--
740+
-- Test some corner cases that have been known to confuse the planner
741+
--
742+
-- ORDER BY on a constant doesn't really need any sorting
743+
SELECT 1 AS x ORDER BY x;
744+
x
745+
---
746+
1
747+
(1 row)
748+
749+
-- But ORDER BY on a set-valued expression does
750+
create function sillysrf(int) returns setof int as
751+
'values (1),(10),(2),($1)' language sql immutable;
752+
select sillysrf(42);
753+
sillysrf
754+
----------
755+
1
756+
10
757+
2
758+
42
759+
(4 rows)
760+
761+
select sillysrf(-1) order by 1;
762+
sillysrf
763+
----------
764+
-1
765+
1
766+
2
767+
10
768+
(4 rows)
769+
770+
drop function sillysrf(int);

src/test/regress/sql/select.sql

+16
Original file line numberDiff line numberDiff line change
@@ -186,3 +186,19 @@ SELECT * FROM foo ORDER BY f1;
186186
SELECT * FROM foo ORDER BY f1 NULLS FIRST;
187187
SELECT * FROM foo ORDER BY f1 DESC;
188188
SELECT * FROM foo ORDER BY f1 DESC NULLS LAST;
189+
190+
--
191+
-- Test some corner cases that have been known to confuse the planner
192+
--
193+
194+
-- ORDER BY on a constant doesn't really need any sorting
195+
SELECT 1 AS x ORDER BY x;
196+
197+
-- But ORDER BY on a set-valued expression does
198+
create function sillysrf(int) returns setof int as
199+
'values (1),(10),(2),($1)' language sql immutable;
200+
201+
select sillysrf(42);
202+
select sillysrf(-1) order by 1;
203+
204+
drop function sillysrf(int);

0 commit comments

Comments
 (0)