diff --git a/src/backend/parser/parse_collate.c b/src/backend/parser/parse_collate.c index 7ed50cd..f432360 100644 --- a/src/backend/parser/parse_collate.c +++ b/src/backend/parser/parse_collate.c @@ -319,86 +319,6 @@ assign_collations_walker(Node *node, assign_collations_context *context) } } break; - case T_CaseExpr: - { - /* - * CaseExpr is a special case because we do not want to - * recurse into the test expression (if any). It was already - * marked with collations during transformCaseExpr, and - * furthermore its collation is not relevant to the result of - * the CASE --- only the output expressions are. So we can't - * use expression_tree_walker here. - */ - CaseExpr *expr = (CaseExpr *) node; - Oid typcollation; - ListCell *lc; - - foreach(lc, expr->args) - { - CaseWhen *when = (CaseWhen *) lfirst(lc); - - Assert(IsA(when, CaseWhen)); - - /* - * The condition expressions mustn't affect the CASE's - * result collation either; but since they are known to - * yield boolean, it's safe to recurse directly on them - * --- they won't change loccontext. - */ - (void) assign_collations_walker((Node *) when->expr, - &loccontext); - (void) assign_collations_walker((Node *) when->result, - &loccontext); - } - (void) assign_collations_walker((Node *) expr->defresult, - &loccontext); - - /* - * Now determine the CASE's output collation. This is the - * same as the general case below. - */ - typcollation = get_typcollation(exprType(node)); - if (OidIsValid(typcollation)) - { - /* Node's result is collatable; what about its input? */ - if (loccontext.strength > COLLATE_NONE) - { - /* Collation state bubbles up from children. */ - collation = loccontext.collation; - strength = loccontext.strength; - location = loccontext.location; - } - else - { - /* - * Collatable output produced without any collatable - * input. Use the type's collation (which is usually - * DEFAULT_COLLATION_OID, but might be different for a - * domain). - */ - collation = typcollation; - strength = COLLATE_IMPLICIT; - location = exprLocation(node); - } - } - else - { - /* Node's result type isn't collatable. */ - collation = InvalidOid; - strength = COLLATE_NONE; - location = -1; /* won't be used */ - } - - /* - * Save the state into the expression node. We know it - * doesn't care about input collation. - */ - if (strength == COLLATE_CONFLICT) - exprSetCollation(node, InvalidOid); - else - exprSetCollation(node, collation); - } - break; case T_RowExpr: { /* @@ -634,9 +554,83 @@ assign_collations_walker(Node *node, assign_collations_context *context) */ Oid typcollation; - (void) expression_tree_walker(node, - assign_collations_walker, - (void *) &loccontext); + /* + * Some node types use part of the general case with + * somepecial processing. Do them here rather than + * duplicate code. + */ + switch (nodeTag(node)) + { + case T_CaseExpr: + { + /* + * CaseExpr is a special case because we + * do not want to recurse into the test + * expression (if any). It was already + * marked with collations during + * transformCaseExpr, and furthermore its + * collation is not relevant to the result + * of the CASE --- only the output + * expressions are. So we can't use + * expression_tree_walker here. + */ + CaseExpr *expr = (CaseExpr *) node; + ListCell *lc; + + foreach(lc, expr->args) + { + CaseWhen *when = (CaseWhen *) lfirst(lc); + + Assert(IsA(when, CaseWhen)); + + /* + * The condition expressions mustn't + * affect the CASE's result collation + * either; but since they are known to + * yield boolean, it's safe to recurse + * directly on them. They won't + * change loccontext. + */ + (void) assign_collations_walker((Node *) when->expr, + &loccontext); + (void) assign_collations_walker((Node *) when->result, + &loccontext); + } + (void) assign_collations_walker((Node *) expr->defresult, + &loccontext); + } + break; + + case T_Aggref: + { + /* + * Aggref is special because expressions + * used only for ordering shouldn't be + * taken to conflict with each other or + * with other args. + */ + + Aggref *aggref = (Aggref *) node; + ListCell *lc; + + foreach (lc, aggref->args) + { + TargetEntry *tle = (TargetEntry *) lfirst(lc); + + if (tle->resjunk) + assign_expr_collations(context->pstate, (Node *) tle); + else + (void) assign_collations_walker((Node *) tle, + &loccontext); + } + } + break; + + default: + (void) expression_tree_walker(node, + assign_collations_walker, + (void *) &loccontext); + } typcollation = get_typcollation(exprType(node)); if (OidIsValid(typcollation)) diff --git a/src/test/regress/expected/collate.out b/src/test/regress/expected/collate.out index 4ab9566..5ad2705 100644 --- a/src/test/regress/expected/collate.out +++ b/src/test/regress/expected/collate.out @@ -362,6 +362,13 @@ SELECT array_agg(b ORDER BY b) FROM collate_test2; {ABD,Abc,abc,bbc} (1 row) +-- Make sure multiple COLLATEs work correctly in aggregates with ORDER BY. +SELECT array_agg(a COLLATE "C" ORDER BY b COLLATE "POSIX") FROM (VALUES ('foo','bar')) v(a,b); + array_agg +----------- + {foo} +(1 row) + SELECT a, b FROM collate_test1 UNION ALL SELECT a, b FROM collate_test1 ORDER BY 2; a | b ---+----- diff --git a/src/test/regress/sql/collate.sql b/src/test/regress/sql/collate.sql index 3c960e7..c32d042 100644 --- a/src/test/regress/sql/collate.sql +++ b/src/test/regress/sql/collate.sql @@ -128,6 +128,9 @@ SELECT min(b), max(b) FROM collate_test2; SELECT array_agg(b ORDER BY b) FROM collate_test1; SELECT array_agg(b ORDER BY b) FROM collate_test2; +-- Make sure multiple COLLATEs work correctly in aggregates with ORDER BY. +SELECT array_agg(a COLLATE "C" ORDER BY b COLLATE "POSIX") FROM (VALUES ('foo','bar')) v(a,b); + SELECT a, b FROM collate_test1 UNION ALL SELECT a, b FROM collate_test1 ORDER BY 2; SELECT a, b FROM collate_test2 UNION SELECT a, b FROM collate_test2 ORDER BY 2; SELECT a, b FROM collate_test2 WHERE a < 4 INTERSECT SELECT a, b FROM collate_test2 WHERE a > 1 ORDER BY 2;