Avoid a premature coercion failure in transformSetOperationTree() when

presented with an UNKNOWN-type Var, which can happen in cases where an
unknown literal appeared in a subquery.  While many such cases will fail
later on anyway in the planner, there are some cases where the planner is
able to flatten the query and replace the Var by the constant before it has
to coerce the union column to the final type.  I had added this check in 8.4
to provide earlier/better error detection, but it causes a regression for
some cases that worked OK before.  Fix by not making the check if the input
node is UNKNOWN type and not a Const or Param.  If it isn't going to work,
it will fail anyway at plan time, with the only real loss being inability to
provide an error cursor.  Per gripe from Britt Piehler.

In passing, rename a couple of variables to remove confusion from an
inner scope masking the same variable names in an outer scope.

diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 0f47e7bae9b98f2bab4581ef5ce82f1e2dcf6d1a..408e2a5a40b43262aa40c7a40126f74dd291c986 100644 (file)
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -17,7 +17,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *     $PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.397 2009/12/15 17:57:47 tgl Exp $
+ *     $PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.398 2009/12/16 22:24:13 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1533,20 +1533,20 @@ transformSetOperationTree(ParseState *pstate, SelectStmt *stmt,
                op->groupClauses = NIL;
                forboth(lci, lcolinfo, rci, rcolinfo)
                {
-                       Node       *lcolinfo = (Node *) lfirst(lci);
-                       Node       *rcolinfo = (Node *) lfirst(rci);
-                       Oid                     lcoltype = exprType(lcolinfo);
-                       Oid                     rcoltype = exprType(rcolinfo);
-                       int32           lcoltypmod = exprTypmod(lcolinfo);
-                       int32           rcoltypmod = exprTypmod(rcolinfo);
+                       Node       *lcolnode = (Node *) lfirst(lci);
+                       Node       *rcolnode = (Node *) lfirst(rci);
+                       Oid                     lcoltype = exprType(lcolnode);
+                       Oid                     rcoltype = exprType(rcolnode);
+                       int32           lcoltypmod = exprTypmod(lcolnode);
+                       int32           rcoltypmod = exprTypmod(rcolnode);
                        Node       *bestexpr;
-                       SetToDefault *rescolinfo;
+                       SetToDefault *rescolnode;
                        Oid                     rescoltype;
                        int32           rescoltypmod;
 
                        /* select common type, same as CASE et al */
                        rescoltype = select_common_type(pstate,
-                                                                                       list_make2(lcolinfo, rcolinfo),
+                                                                                       list_make2(lcolnode, rcolnode),
                                                                                        context,
                                                                                        &bestexpr);
                        /* if same type and same typmod, use typmod; else default */
@@ -1555,18 +1555,35 @@ transformSetOperationTree(ParseState *pstate, SelectStmt *stmt,
                        else
                                rescoltypmod = -1;
 
-                       /* verify the coercions are actually possible */
-                       (void) coerce_to_common_type(pstate, lcolinfo,
-                                                                                rescoltype, context);
-                       (void) coerce_to_common_type(pstate, rcolinfo,
-                                                                                rescoltype, context);
+                       /*
+                        * Verify the coercions are actually possible.  If not, we'd
+                        * fail later anyway, but we want to fail now while we have
+                        * sufficient context to produce an error cursor position.
+                        *
+                        * The if-tests might look wrong, but they are correct: we should
+                        * verify if the input is non-UNKNOWN *or* if it is an UNKNOWN
+                        * Const (to verify the literal is valid for the target data type)
+                        * or Param (to possibly resolve the Param's type).  We should do
+                        * nothing if the input is say an UNKNOWN Var, which can happen in
+                        * some cases.  The planner is sometimes able to fold the Var to a
+                        * constant before it has to coerce the type, so failing now would
+                        * just break cases that might work.
+                        */
+                       if (lcoltype != UNKNOWNOID ||
+                               IsA(lcolnode, Const) || IsA(lcolnode, Param))
+                               (void) coerce_to_common_type(pstate, lcolnode,
+                                                                                        rescoltype, context);
+                       if (rcoltype != UNKNOWNOID ||
+                               IsA(rcolnode, Const) || IsA(rcolnode, Param))
+                               (void) coerce_to_common_type(pstate, rcolnode,
+                                                                                        rescoltype, context);
 
                        /* emit results */
-                       rescolinfo = makeNode(SetToDefault);
-                       rescolinfo->typeId = rescoltype;
-                       rescolinfo->typeMod = rescoltypmod;
-                       rescolinfo->location = exprLocation(bestexpr);
-                       *colInfo = lappend(*colInfo, rescolinfo);
+                       rescolnode = makeNode(SetToDefault);
+                       rescolnode->typeId = rescoltype;
+                       rescolnode->typeMod = rescoltypmod;
+                       rescolnode->location = exprLocation(bestexpr);
+                       *colInfo = lappend(*colInfo, rescolnode);
 
                        op->colTypes = lappend_oid(op->colTypes, rescoltype);
                        op->colTypmods = lappend_int(op->colTypmods, rescoltypmod);
@@ -1584,7 +1601,7 @@ transformSetOperationTree(ParseState *pstate, SelectStmt *stmt,
                                ParseCallbackState pcbstate;
 
                                setup_parser_errposition_callback(&pcbstate, pstate,
-                                                                                                 rescolinfo->location);
+                                                                                                 rescolnode->location);
 
                                /* determine the eqop and optional sortop */
                                get_sort_group_operators(rescoltype,

diff --git a/src/test/regress/expected/union.out b/src/test/regress/expected/union.out
index be046a9b63200ea2a3b9a4fdd2aab3a98e31d329..2f8037f9eb04dae009f90e58bde11736d877cad2 100644 (file)
--- a/src/test/regress/expected/union.out
+++ b/src/test/regress/expected/union.out
@@ -433,3 +433,25 @@ SELECT q1 FROM int8_tbl EXCEPT (((SELECT q2 FROM int8_tbl ORDER BY q2 LIMIT 1)))
  4567890123456789 | -4567890123456789
 (5 rows)
 
+--
+-- Check handling of a case with unknown constants.  We don't guarantee
+-- an undecorated constant will work in all cases, but historically this
+-- usage has worked, so test we don't break it.
+--
+SELECT a.f1 FROM (SELECT 'test' AS f1 FROM varchar_tbl) a
+UNION
+SELECT b.f1 FROM (SELECT f1 FROM varchar_tbl) b
+ORDER BY 1;
+  f1  
+------
+ a
+ ab
+ abcd
+ test
+(4 rows)
+
+-- This should fail, but it should produce an error cursor
+SELECT '3.4'::numeric UNION SELECT 'foo';
+ERROR:  invalid input syntax for type numeric: "foo"
+LINE 1: SELECT '3.4'::numeric UNION SELECT 'foo';
+                                           ^

diff --git a/src/test/regress/sql/union.sql b/src/test/regress/sql/union.sql
index 81e19d165c43aad1e1a4f2b3f9b6d87dba5efd97..daa72c929c034de52be43e198674991ef28ca5f9 100644 (file)
--- a/src/test/regress/sql/union.sql
+++ b/src/test/regress/sql/union.sql
@@ -153,4 +153,16 @@ SELECT q1 FROM int8_tbl EXCEPT (((SELECT q2 FROM int8_tbl ORDER BY q2 LIMIT 1)))
 
 (((((select * from int8_tbl)))));
 
+--
+-- Check handling of a case with unknown constants.  We don't guarantee
+-- an undecorated constant will work in all cases, but historically this
+-- usage has worked, so test we don't break it.
+--
+
+SELECT a.f1 FROM (SELECT 'test' AS f1 FROM varchar_tbl) a
+UNION
+SELECT b.f1 FROM (SELECT f1 FROM varchar_tbl) b
+ORDER BY 1;
 
+-- This should fail, but it should produce an error cursor
+SELECT '3.4'::numeric UNION SELECT 'foo';