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.

diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index b6503ef193be72ff32c3332e026a1163a9c49897..fc862438ffb011219d45430832fa840b060fd12c 100644 (file)
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -10,7 +10,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/optimizer/path/equivclass.c,v 1.2 2007/01/22 20:00:39 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/optimizer/path/equivclass.c,v 1.3 2007/07/07 20:46:45 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -328,8 +328,8 @@ add_eq_member(EquivalenceClass *ec, Expr *expr, Relids relids,
                /*
                 * No Vars, assume it's a pseudoconstant.  This is correct for
                 * entries generated from process_equivalence(), because a WHERE
-                * clause can't contain aggregates and non-volatility was checked
-                * before process_equivalence() ever got called.  But
+                * clause can't contain aggregates or SRFs, and non-volatility was
+                * checked before process_equivalence() ever got called.  But
                 * get_eclass_for_sort_expr() has to work harder.  We put the tests
                 * there not here to save cycles in the equivalence case.
                 */
@@ -428,13 +428,15 @@ get_eclass_for_sort_expr(PlannerInfo *root,
                                                  false, expr_datatype);
 
        /*
-        * add_eq_member doesn't check for volatile functions or aggregates,
-        * but such could appear in sort expressions, so we have to check
-        * whether its const-marking was correct.
+        * add_eq_member doesn't check for volatile functions, set-returning
+        * functions, or aggregates, but such could appear in sort expressions;
+        * so we have to check whether its const-marking was correct.
         */
        if (newec->ec_has_const)
        {
-               if (newec->ec_has_volatile || contain_agg_clause((Node *) expr))
+               if (newec->ec_has_volatile ||
+                       expression_returns_set((Node *) expr) ||
+                       contain_agg_clause((Node *) expr))
                {
                        newec->ec_has_const = false;
                        newem->em_is_const = false;

diff --git a/src/backend/optimizer/plan/planmain.c b/src/backend/optimizer/plan/planmain.c
index c31f7d5aa656884cff392d828eb86c653715b0d5..f8ff3a71508ce6f94f6694f98eaf0ada29de063d 100644 (file)
--- a/src/backend/optimizer/plan/planmain.c
+++ b/src/backend/optimizer/plan/planmain.c
@@ -14,7 +14,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/optimizer/plan/planmain.c,v 1.101 2007/05/04 01:13:44 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/optimizer/plan/planmain.c,v 1.102 2007/07/07 20:46:45 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -106,9 +106,21 @@ query_planner(PlannerInfo *root, List *tlist,
         */
        if (parse->jointree->fromlist == NIL)
        {
+               /* We need a trivial path result */
                *cheapest_path = (Path *)
                        create_result_path((List *) parse->jointree->quals);
                *sorted_path = NULL;
+               /*
+                * We still are required to canonicalize any pathkeys, in case
+                * it's something like "SELECT 2+2 ORDER BY 1".
+                */
+               root->canon_pathkeys = NIL;
+               root->query_pathkeys = canonicalize_pathkeys(root,
+                                                                                                        root->query_pathkeys);
+               root->group_pathkeys = canonicalize_pathkeys(root,
+                                                                                                        root->group_pathkeys);
+               root->sort_pathkeys = canonicalize_pathkeys(root,
+                                                                                                       root->sort_pathkeys);
                return;
        }
 

diff --git a/src/test/regress/expected/select.out b/src/test/regress/expected/select.out
index d58c8d2811be2f598d9ae25819f5cb4f63b6476a..2936f0306aba92ea92d6f12a59c66a01b195d785 100644 (file)
--- a/src/test/regress/expected/select.out
+++ b/src/test/regress/expected/select.out
@@ -736,3 +736,35 @@ SELECT * FROM foo ORDER BY f1 DESC NULLS LAST;
    
 (7 rows)
 
+--
+-- Test some corner cases that have been known to confuse the planner
+--
+-- ORDER BY on a constant doesn't really need any sorting
+SELECT 1 AS x ORDER BY x;
+ x 
+---
+ 1
+(1 row)
+
+-- But ORDER BY on a set-valued expression does
+create function sillysrf(int) returns setof int as
+  'values (1),(10),(2),($1)' language sql immutable;
+select sillysrf(42);
+ sillysrf 
+----------
+        1
+       10
+        2
+       42
+(4 rows)
+
+select sillysrf(-1) order by 1;
+ sillysrf 
+----------
+       -1
+        1
+        2
+       10
+(4 rows)
+
+drop function sillysrf(int);

diff --git a/src/test/regress/sql/select.sql b/src/test/regress/sql/select.sql
index 8c92168c9b88f08276874514617f114d02b4e5ea..bb1fc9b9347d0e9db40550aa9cd01a4721f1b984 100644 (file)
--- a/src/test/regress/sql/select.sql
+++ b/src/test/regress/sql/select.sql
@@ -186,3 +186,19 @@ SELECT * FROM foo ORDER BY f1;
 SELECT * FROM foo ORDER BY f1 NULLS FIRST;
 SELECT * FROM foo ORDER BY f1 DESC;
 SELECT * FROM foo ORDER BY f1 DESC NULLS LAST;
+
+--
+-- Test some corner cases that have been known to confuse the planner
+--
+
+-- ORDER BY on a constant doesn't really need any sorting
+SELECT 1 AS x ORDER BY x;
+
+-- But ORDER BY on a set-valued expression does
+create function sillysrf(int) returns setof int as
+  'values (1),(10),(2),($1)' language sql immutable;
+
+select sillysrf(42);
+select sillysrf(-1) order by 1;
+
+drop function sillysrf(int);