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.

3 files changed, 71 insertions, 20 deletions

diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 0f47e7bae9..408e2a5a40 100644
--- 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 be046a9b63..2f8037f9eb 100644
--- 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 81e19d165c..daa72c929c 100644
--- 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';