diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2012-01-02 14:43:51 -0500 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2012-01-02 14:43:51 -0500 |
commit | 188f1b928205bf33ce29887eeeee26ce9227908f (patch) | |
tree | 37b742f21a639411ff5c6e8d2bb8e9123bf53860 | |
parent | 71b23708d4433d731082ed9c5ca491c7595e0e4d (diff) | |
download | postgresql-188f1b928205bf33ce29887eeeee26ce9227908f.tar.gz |
Fix coerce_to_target_type for coerce_type's klugy handling of COLLATE.
Because coerce_type recurses into the argument of a CollateExpr,
coerce_to_target_type's longstanding code for detecting whether coerce_type
had actually done anything (to wit, returned a different node than it
passed in) was broken in 9.1. This resulted in unexpected failures in
hide_coercion_node; which was not the latter's fault, since it's critical
that we never call it on anything that wasn't inserted by coerce_type.
(Else we might decide to "hide" a user-written function call.)
Fix by removing and replacing the CollateExpr in coerce_to_target_type
itself. This is all pretty ugly but I don't immediately see a way to make
it nicer.
Per report from Jean-Yves F. Barbier.
-rw-r--r-- | src/backend/parser/parse_coerce.c | 28 | ||||
-rw-r--r-- | src/test/regress/expected/collate.out | 3 | ||||
-rw-r--r-- | src/test/regress/sql/collate.sql | 5 |
3 files changed, 35 insertions, 1 deletions
diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c index f26c69abdd..bfd379323a 100644 --- a/src/backend/parser/parse_coerce.c +++ b/src/backend/parser/parse_coerce.c @@ -81,10 +81,24 @@ coerce_to_target_type(ParseState *pstate, Node *expr, Oid exprtype, int location) { Node *result; + Node *origexpr; if (!can_coerce_type(1, &exprtype, &targettype, ccontext)) return NULL; + /* + * If the input has a CollateExpr at the top, strip it off, perform the + * coercion, and put a new one back on. This is annoying since it + * duplicates logic in coerce_type, but if we don't do this then it's too + * hard to tell whether coerce_type actually changed anything, and we + * *must* know that to avoid possibly calling hide_coercion_node on + * something that wasn't generated by coerce_type. Note that if there are + * multiple stacked CollateExprs, we just discard all but the topmost. + */ + origexpr = expr; + while (expr && IsA(expr, CollateExpr)) + expr = (Node *) ((CollateExpr *) expr)->arg; + result = coerce_type(pstate, expr, exprtype, targettype, targettypmod, ccontext, cformat, location); @@ -100,6 +114,18 @@ coerce_to_target_type(ParseState *pstate, Node *expr, Oid exprtype, (cformat != COERCE_IMPLICIT_CAST), (result != expr && !IsA(result, Const))); + if (expr != origexpr) + { + /* Reinstall top CollateExpr */ + CollateExpr *coll = (CollateExpr *) origexpr; + CollateExpr *newcoll = makeNode(CollateExpr); + + newcoll->arg = (Expr *) result; + newcoll->collOid = coll->collOid; + newcoll->location = coll->location; + result = (Node *) newcoll; + } + return result; } @@ -318,7 +344,7 @@ coerce_type(ParseState *pstate, Node *node, * If we have a COLLATE clause, we have to push the coercion * underneath the COLLATE. This is really ugly, but there is little * choice because the above hacks on Consts and Params wouldn't happen - * otherwise. + * otherwise. This kluge has consequences in coerce_to_target_type. */ CollateExpr *coll = (CollateExpr *) node; CollateExpr *newcoll = makeNode(CollateExpr); diff --git a/src/test/regress/expected/collate.out b/src/test/regress/expected/collate.out index dc17feaefe..a15e6911b0 100644 --- a/src/test/regress/expected/collate.out +++ b/src/test/regress/expected/collate.out @@ -574,6 +574,9 @@ ALTER TABLE collate_test22 ADD FOREIGN KEY (f2) REFERENCES collate_test20; RESET enable_seqscan; RESET enable_hashjoin; RESET enable_nestloop; +-- 9.1 bug with useless COLLATE in an expression subject to length coercion +CREATE TEMP TABLE vctable (f1 varchar(25)); +INSERT INTO vctable VALUES ('foo' COLLATE "C"); -- -- Clean up. Many of these table names will be re-used if the user is -- trying to run any platform-specific collation tests later, so we diff --git a/src/test/regress/sql/collate.sql b/src/test/regress/sql/collate.sql index 52d830d58a..f72f3edb9d 100644 --- a/src/test/regress/sql/collate.sql +++ b/src/test/regress/sql/collate.sql @@ -214,6 +214,11 @@ RESET enable_seqscan; RESET enable_hashjoin; RESET enable_nestloop; +-- 9.1 bug with useless COLLATE in an expression subject to length coercion + +CREATE TEMP TABLE vctable (f1 varchar(25)); +INSERT INTO vctable VALUES ('foo' COLLATE "C"); + -- -- Clean up. Many of these table names will be re-used if the user is -- trying to run any platform-specific collation tests later, so we |