Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit41a2760

Browse files
committed
Fix collation assignment for aggregates with ORDER BY.
ORDER BY expressions were being treated the same as regular aggregatearguments for purposes of collation determination, but really they shouldnot affect the aggregate's collation at all; only collations of theaggregate's regular arguments should affect it.In many cases this mistake would lead to incorrectly throwing a "collationconflict" error; but in some cases the corrected code will silently assigna different collation to the aggregate than before, for exampleagg(foo ORDER BY bar COLLATE "x")which will now use foo's collation rather than "x" for the aggregate.Given this risk and the lack of field complaints about the issue, itdoesn't seem prudent to back-patch.In passing, rearrange code in assign_collations_walker so that we don'tneed multiple copies of the standard logic for computing collation of anode with children. (Previously, CaseExpr duplicated the standard logic,and we would have needed a third copy for Aggref without this change.)Andrew Gierth and David Fetter
1 parentb42ea79 commit41a2760

File tree

3 files changed

+121
-84
lines changed

3 files changed

+121
-84
lines changed

‎src/backend/parser/parse_collate.c

Lines changed: 93 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -319,86 +319,6 @@ assign_collations_walker(Node *node, assign_collations_context *context)
319319
}
320320
}
321321
break;
322-
caseT_CaseExpr:
323-
{
324-
/*
325-
* CaseExpr is a special case because we do not want to
326-
* recurse into the test expression (if any). It was already
327-
* marked with collations during transformCaseExpr, and
328-
* furthermore its collation is not relevant to the result of
329-
* the CASE --- only the output expressions are. So we can't
330-
* use expression_tree_walker here.
331-
*/
332-
CaseExpr*expr= (CaseExpr*)node;
333-
Oidtypcollation;
334-
ListCell*lc;
335-
336-
foreach(lc,expr->args)
337-
{
338-
CaseWhen*when= (CaseWhen*)lfirst(lc);
339-
340-
Assert(IsA(when,CaseWhen));
341-
342-
/*
343-
* The condition expressions mustn't affect the CASE's
344-
* result collation either; but since they are known to
345-
* yield boolean, it's safe to recurse directly on them
346-
* --- they won't change loccontext.
347-
*/
348-
(void)assign_collations_walker((Node*)when->expr,
349-
&loccontext);
350-
(void)assign_collations_walker((Node*)when->result,
351-
&loccontext);
352-
}
353-
(void)assign_collations_walker((Node*)expr->defresult,
354-
&loccontext);
355-
356-
/*
357-
* Now determine the CASE's output collation. This is the
358-
* same as the general case below.
359-
*/
360-
typcollation=get_typcollation(exprType(node));
361-
if (OidIsValid(typcollation))
362-
{
363-
/* Node's result is collatable; what about its input? */
364-
if (loccontext.strength>COLLATE_NONE)
365-
{
366-
/* Collation state bubbles up from children. */
367-
collation=loccontext.collation;
368-
strength=loccontext.strength;
369-
location=loccontext.location;
370-
}
371-
else
372-
{
373-
/*
374-
* Collatable output produced without any collatable
375-
* input. Use the type's collation (which is usually
376-
* DEFAULT_COLLATION_OID, but might be different for a
377-
* domain).
378-
*/
379-
collation=typcollation;
380-
strength=COLLATE_IMPLICIT;
381-
location=exprLocation(node);
382-
}
383-
}
384-
else
385-
{
386-
/* Node's result type isn't collatable. */
387-
collation=InvalidOid;
388-
strength=COLLATE_NONE;
389-
location=-1;/* won't be used */
390-
}
391-
392-
/*
393-
* Save the state into the expression node. We know it
394-
* doesn't care about input collation.
395-
*/
396-
if (strength==COLLATE_CONFLICT)
397-
exprSetCollation(node,InvalidOid);
398-
else
399-
exprSetCollation(node,collation);
400-
}
401-
break;
402322
caseT_RowExpr:
403323
{
404324
/*
@@ -630,14 +550,103 @@ assign_collations_walker(Node *node, assign_collations_context *context)
630550
{
631551
/*
632552
* General case for most expression nodes with children. First
633-
* recurse, then figure out what to assignhere.
553+
* recurse, then figure out what to assignto this node.
634554
*/
635555
Oidtypcollation;
636556

637-
(void)expression_tree_walker(node,
638-
assign_collations_walker,
639-
(void*)&loccontext);
557+
/*
558+
* For most node types, we want to treat all the child
559+
* expressions alike; but there are a few exceptions, hence
560+
* this inner switch.
561+
*/
562+
switch (nodeTag(node))
563+
{
564+
caseT_Aggref:
565+
{
566+
/*
567+
* Aggref is a special case because expressions
568+
* used only for ordering shouldn't be taken to
569+
* conflict with each other or with regular args.
570+
* So we apply assign_expr_collations() to them
571+
* rather than passing down our loccontext.
572+
*
573+
* Note that we recurse to each TargetEntry, not
574+
* directly to its contained expression, so that
575+
* the case above for T_TargetEntry will apply
576+
* appropriate checks to agg ORDER BY items.
577+
*
578+
* We need not recurse into the aggorder or
579+
* aggdistinct lists, because those contain only
580+
* SortGroupClause nodes which we need not
581+
* process.
582+
*/
583+
Aggref*aggref= (Aggref*)node;
584+
ListCell*lc;
585+
586+
foreach(lc,aggref->args)
587+
{
588+
TargetEntry*tle= (TargetEntry*)lfirst(lc);
589+
590+
Assert(IsA(tle,TargetEntry));
591+
if (tle->resjunk)
592+
assign_expr_collations(context->pstate,
593+
(Node*)tle);
594+
else
595+
(void)assign_collations_walker((Node*)tle,
596+
&loccontext);
597+
}
598+
}
599+
break;
600+
caseT_CaseExpr:
601+
{
602+
/*
603+
* CaseExpr is a special case because we do not
604+
* want to recurse into the test expression (if
605+
* any). It was already marked with collations
606+
* during transformCaseExpr, and furthermore its
607+
* collation is not relevant to the result of the
608+
* CASE --- only the output expressions are.
609+
*/
610+
CaseExpr*expr= (CaseExpr*)node;
611+
ListCell*lc;
612+
613+
foreach(lc,expr->args)
614+
{
615+
CaseWhen*when= (CaseWhen*)lfirst(lc);
616+
617+
Assert(IsA(when,CaseWhen));
618+
619+
/*
620+
* The condition expressions mustn't affect
621+
* the CASE's result collation either; but
622+
* since they are known to yield boolean, it's
623+
* safe to recurse directly on them --- they
624+
* won't change loccontext.
625+
*/
626+
(void)assign_collations_walker((Node*)when->expr,
627+
&loccontext);
628+
(void)assign_collations_walker((Node*)when->result,
629+
&loccontext);
630+
}
631+
(void)assign_collations_walker((Node*)expr->defresult,
632+
&loccontext);
633+
}
634+
break;
635+
default:
636+
637+
/*
638+
* Normal case: all child expressions contribute
639+
* equally to loccontext.
640+
*/
641+
(void)expression_tree_walker(node,
642+
assign_collations_walker,
643+
(void*)&loccontext);
644+
break;
645+
}
640646

647+
/*
648+
* Now figure out what collation to assign to this node.
649+
*/
641650
typcollation=get_typcollation(exprType(node));
642651
if (OidIsValid(typcollation))
643652
{

‎src/test/regress/expected/collate.out

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,28 @@ SELECT array_agg(b ORDER BY b) FROM collate_test2;
362362
{ABD,Abc,abc,bbc}
363363
(1 row)
364364

365+
-- In aggregates, ORDER BY expressions don't affect aggregate's collation
366+
SELECT string_agg(x COLLATE "C", y COLLATE "POSIX") FROM collate_test10; -- fail
367+
ERROR: collation mismatch between explicit collations "C" and "POSIX"
368+
LINE 1: SELECT string_agg(x COLLATE "C", y COLLATE "POSIX") FROM col...
369+
^
370+
SELECT array_agg(x COLLATE "C" ORDER BY y COLLATE "POSIX") FROM collate_test10;
371+
array_agg
372+
-----------
373+
{HIJ,hij}
374+
(1 row)
375+
376+
SELECT array_agg(a ORDER BY x COLLATE "C", y COLLATE "POSIX") FROM collate_test10;
377+
array_agg
378+
-----------
379+
{2,1}
380+
(1 row)
381+
382+
SELECT array_agg(a ORDER BY x||y) FROM collate_test10; -- fail
383+
ERROR: collation mismatch between implicit collations "C" and "POSIX"
384+
LINE 1: SELECT array_agg(a ORDER BY x||y) FROM collate_test10;
385+
^
386+
HINT: You can choose the collation by applying the COLLATE clause to one or both expressions.
365387
SELECT a, b FROM collate_test1 UNION ALL SELECT a, b FROM collate_test1 ORDER BY 2;
366388
a | b
367389
---+-----

‎src/test/regress/sql/collate.sql

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,12 @@ SELECT min(b), max(b) FROM collate_test2;
128128
SELECT array_agg(bORDER BY b)FROM collate_test1;
129129
SELECT array_agg(bORDER BY b)FROM collate_test2;
130130

131+
-- In aggregates, ORDER BY expressions don't affect aggregate's collation
132+
SELECT string_agg(x COLLATE"C", y COLLATE"POSIX")FROM collate_test10;-- fail
133+
SELECT array_agg(x COLLATE"C"ORDER BY y COLLATE"POSIX")FROM collate_test10;
134+
SELECT array_agg(aORDER BY x COLLATE"C", y COLLATE"POSIX")FROM collate_test10;
135+
SELECT array_agg(aORDER BY x||y)FROM collate_test10;-- fail
136+
131137
SELECT a, bFROM collate_test1UNION ALLSELECT a, bFROM collate_test1ORDER BY2;
132138
SELECT a, bFROM collate_test2UNIONSELECT a, bFROM collate_test2ORDER BY2;
133139
SELECT a, bFROM collate_test2WHERE a<4 INTERSECTSELECT a, bFROM collate_test2WHERE a>1ORDER BY2;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp