From a19002d4e5da028ff7280554b281e402c609898b Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 9 Apr 2011 14:40:09 -0400 Subject: [PATCH] Adjust collation determination rules as per discussion. Remove crude hack that tried to propagate collation through a function-returning-record, ie, from the function's arguments to individual fields selected from its result record. That is just plain inconsistent, because the function result is composite and cannot have a collation; and there's no hope of making this kind of action-at-a-distance work consistently. Adjust regression test cases that expected this to happen. Meanwhile, the behavior of casting to a domain with a declared collation stays the same as it was, since that seemed to be the consensus. --- src/backend/parser/parse_collate.c | 27 ++--------- src/backend/parser/parse_func.c | 2 +- src/backend/parser/parse_target.c | 2 +- .../regress/expected/collate.linux.utf8.out | 46 +++++++++---------- src/test/regress/expected/collate.out | 32 ++++++------- src/test/regress/sql/collate.linux.utf8.sql | 10 ++-- src/test/regress/sql/collate.sql | 8 ++-- 7 files changed, 55 insertions(+), 72 deletions(-) diff --git a/src/backend/parser/parse_collate.c b/src/backend/parser/parse_collate.c index 8860679ccb..3e557db266 100644 --- a/src/backend/parser/parse_collate.c +++ b/src/backend/parser/parse_collate.c @@ -289,10 +289,11 @@ assign_collations_walker(Node *node, assign_collations_context *context) case T_FieldSelect: { /* - * FieldSelect is a special case because the field may have - * a non-default collation, in which case we should use that. - * The field's collation was already looked up and saved - * in the node. + * For FieldSelect, the result has the field's declared + * collation, independently of what happened in the arguments. + * (The immediate argument must be composite and thus not + * collatable, anyhow.) The field's collation was already + * looked up and saved in the node. */ FieldSelect *expr = (FieldSelect *) node; @@ -304,24 +305,6 @@ assign_collations_walker(Node *node, assign_collations_context *context) if (OidIsValid(expr->resultcollid)) { /* Node's result type is collatable. */ - if (expr->resultcollid == DEFAULT_COLLATION_OID) - { - /* - * The immediate input node necessarily yields a - * composite type, so it will have no exposed - * collation. However, if we are selecting a field - * from a function returning composite, see if we - * can bubble up a collation from the function's - * input. XXX this is a bit of a hack, rethink ... - */ - if (IsA(expr->arg, FuncExpr)) - { - FuncExpr *fexpr = (FuncExpr *) expr->arg; - - if (OidIsValid(fexpr->inputcollid)) - expr->resultcollid = fexpr->inputcollid; - } - } /* Pass up field's collation as an implicit choice. */ collation = expr->resultcollid; strength = COLLATE_IMPLICIT; diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c index a187287e28..ba699e9a1e 100644 --- a/src/backend/parser/parse_func.c +++ b/src/backend/parser/parse_func.c @@ -1384,7 +1384,7 @@ ParseComplexProjection(ParseState *pstate, char *funcname, Node *first_arg, fselect->fieldnum = i + 1; fselect->resulttype = att->atttypid; fselect->resulttypmod = att->atttypmod; - /* resultcollid may get overridden by parse_collate.c */ + /* save attribute's collation for parse_collate.c */ fselect->resultcollid = att->attcollation; return (Node *) fselect; } diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c index d53d1d9f00..52c6db2eb5 100644 --- a/src/backend/parser/parse_target.c +++ b/src/backend/parser/parse_target.c @@ -1290,7 +1290,7 @@ ExpandRowReference(ParseState *pstate, Node *expr, fselect->fieldnum = i + 1; fselect->resulttype = att->atttypid; fselect->resulttypmod = att->atttypmod; - /* resultcollid may get overridden by parse_collate.c */ + /* save attribute's collation for parse_collate.c */ fselect->resultcollid = att->attcollation; if (targetlist) diff --git a/src/test/regress/expected/collate.linux.utf8.out b/src/test/regress/expected/collate.linux.utf8.out index a4ec1e9080..fe4d27e34b 100644 --- a/src/test/regress/expected/collate.linux.utf8.out +++ b/src/test/regress/expected/collate.linux.utf8.out @@ -771,33 +771,33 @@ SELECT * FROM unnest((SELECT array_agg(b ORDER BY b) FROM collate_test3)) ORDER äbc (4 rows) -CREATE FUNCTION dup (f1 anyelement, f2 out anyelement, f3 out anyarray) - AS 'select $1, array[$1,$1]' LANGUAGE sql; -SELECT a, (dup(b)).* FROM collate_test1 ORDER BY 2; - a | f2 | f3 ----+-----+----------- - 1 | abc | {abc,abc} - 4 | ABC | {ABC,ABC} - 2 | äbc | {äbc,äbc} - 3 | bbc | {bbc,bbc} +CREATE FUNCTION dup (anyelement) RETURNS anyelement + AS 'select $1' LANGUAGE sql; +SELECT a, dup(b) FROM collate_test1 ORDER BY 2; + a | dup +---+----- + 1 | abc + 4 | ABC + 2 | äbc + 3 | bbc (4 rows) -SELECT a, (dup(b)).* FROM collate_test2 ORDER BY 2; - a | f2 | f3 ----+-----+----------- - 1 | abc | {abc,abc} - 4 | ABC | {ABC,ABC} - 3 | bbc | {bbc,bbc} - 2 | äbc | {äbc,äbc} +SELECT a, dup(b) FROM collate_test2 ORDER BY 2; + a | dup +---+----- + 1 | abc + 4 | ABC + 3 | bbc + 2 | äbc (4 rows) -SELECT a, (dup(b)).* FROM collate_test3 ORDER BY 2; - a | f2 | f3 ----+-----+----------- - 4 | ABC | {ABC,ABC} - 1 | abc | {abc,abc} - 3 | bbc | {bbc,bbc} - 2 | äbc | {äbc,äbc} +SELECT a, dup(b) FROM collate_test3 ORDER BY 2; + a | dup +---+----- + 4 | ABC + 1 | abc + 3 | bbc + 2 | äbc (4 rows) -- indexes diff --git a/src/test/regress/expected/collate.out b/src/test/regress/expected/collate.out index f1a025ae37..230eb9ef31 100644 --- a/src/test/regress/expected/collate.out +++ b/src/test/regress/expected/collate.out @@ -502,24 +502,24 @@ SELECT * FROM unnest((SELECT array_agg(b ORDER BY b) FROM collate_test2)) ORDER bbc (4 rows) -CREATE FUNCTION dup (f1 anyelement, f2 out anyelement, f3 out anyarray) - AS 'select $1, array[$1,$1]' LANGUAGE sql; -SELECT a, (dup(b)).* FROM collate_test1 ORDER BY 2; - a | f2 | f3 ----+-----+----------- - 4 | ABD | {ABD,ABD} - 2 | Abc | {Abc,Abc} - 1 | abc | {abc,abc} - 3 | bbc | {bbc,bbc} +CREATE FUNCTION dup (anyelement) RETURNS anyelement + AS 'select $1' LANGUAGE sql; +SELECT a, dup(b) FROM collate_test1 ORDER BY 2; + a | dup +---+----- + 4 | ABD + 2 | Abc + 1 | abc + 3 | bbc (4 rows) -SELECT a, (dup(b)).* FROM collate_test2 ORDER BY 2; - a | f2 | f3 ----+-----+----------- - 4 | ABD | {ABD,ABD} - 2 | Abc | {Abc,Abc} - 1 | abc | {abc,abc} - 3 | bbc | {bbc,bbc} +SELECT a, dup(b) FROM collate_test2 ORDER BY 2; + a | dup +---+----- + 4 | ABD + 2 | Abc + 1 | abc + 3 | bbc (4 rows) -- indexes diff --git a/src/test/regress/sql/collate.linux.utf8.sql b/src/test/regress/sql/collate.linux.utf8.sql index 265db72220..57f5947aa9 100644 --- a/src/test/regress/sql/collate.linux.utf8.sql +++ b/src/test/regress/sql/collate.linux.utf8.sql @@ -242,12 +242,12 @@ SELECT * FROM unnest((SELECT array_agg(b ORDER BY b) FROM collate_test1)) ORDER SELECT * FROM unnest((SELECT array_agg(b ORDER BY b) FROM collate_test2)) ORDER BY 1; SELECT * FROM unnest((SELECT array_agg(b ORDER BY b) FROM collate_test3)) ORDER BY 1; -CREATE FUNCTION dup (f1 anyelement, f2 out anyelement, f3 out anyarray) - AS 'select $1, array[$1,$1]' LANGUAGE sql; +CREATE FUNCTION dup (anyelement) RETURNS anyelement + AS 'select $1' LANGUAGE sql; -SELECT a, (dup(b)).* FROM collate_test1 ORDER BY 2; -SELECT a, (dup(b)).* FROM collate_test2 ORDER BY 2; -SELECT a, (dup(b)).* FROM collate_test3 ORDER BY 2; +SELECT a, dup(b) FROM collate_test1 ORDER BY 2; +SELECT a, dup(b) FROM collate_test2 ORDER BY 2; +SELECT a, dup(b) FROM collate_test3 ORDER BY 2; -- indexes diff --git a/src/test/regress/sql/collate.sql b/src/test/regress/sql/collate.sql index c904aa77f3..39c22674b9 100644 --- a/src/test/regress/sql/collate.sql +++ b/src/test/regress/sql/collate.sql @@ -168,11 +168,11 @@ SELECT a, CAST(b AS varchar) FROM collate_test2 ORDER BY 2; SELECT * FROM unnest((SELECT array_agg(b ORDER BY b) FROM collate_test1)) ORDER BY 1; SELECT * FROM unnest((SELECT array_agg(b ORDER BY b) FROM collate_test2)) ORDER BY 1; -CREATE FUNCTION dup (f1 anyelement, f2 out anyelement, f3 out anyarray) - AS 'select $1, array[$1,$1]' LANGUAGE sql; +CREATE FUNCTION dup (anyelement) RETURNS anyelement + AS 'select $1' LANGUAGE sql; -SELECT a, (dup(b)).* FROM collate_test1 ORDER BY 2; -SELECT a, (dup(b)).* FROM collate_test2 ORDER BY 2; +SELECT a, dup(b) FROM collate_test1 ORDER BY 2; +SELECT a, dup(b) FROM collate_test2 ORDER BY 2; -- indexes