From ac7a5a3f25708c03242edc301ad008236fc36c7e Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 2 Jan 2012 14:43:45 -0500 Subject: [PATCH] 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. --- src/backend/parser/parse_coerce.c | 28 ++++++++++++++++++++++++++- src/test/regress/expected/collate.out | 3 +++ src/test/regress/sql/collate.sql | 5 +++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c index e727837454..6661a3d8a1 100644 --- a/src/backend/parser/parse_coerce.c +++ b/src/backend/parser/parse_coerce.c @@ -79,10 +79,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); @@ -98,6 +112,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