Folks, While testing the upcoming FILTER clause for aggregates, Erik Rijkers uncovered a long-standing bug in $subject, namely that this case wasn't handled. Please find attached a patch by Andrew Gierth and myself which fixes this issue and adds a regression test to ensure it remains fixed.
Cheers, David. -- David Fetter <da...@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
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;
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
