From 52fc0075ab794136f12847971068cbddba297fe4 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Wed, 16 Dec 2009 22:24:13 +0000
Subject: [PATCH] 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.
---
src/backend/parser/analyze.c | 57 +++++++++++++++++++----------
src/test/regress/expected/union.out | 22 +++++++++++
src/test/regress/sql/union.sql | 12 ++++++
3 files changed, 71 insertions(+), 20 deletions(-)
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 0f47e7bae9b..408e2a5a40b 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 be046a9b632..2f8037f9eb0 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 81e19d165c4..daa72c929c0 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';
--
GitLab