Skip to content
Snippets Groups Projects
Commit 174fab99 authored by Andrew Gierth's avatar Andrew Gierth
Browse files

Postpone aggregate checks until after collation is assigned.

Previously, parseCheckAggregates was run before
assign_query_collations, but this causes problems if any expression
has already had a collation assigned by some transform function (e.g.
transformCaseExpr) before parseCheckAggregates runs. The differing
collations would cause expressions not to be recognized as equal to
the ones in the GROUP BY clause, leading to spurious errors about
unaggregated column references.

The result was that CASE expr WHEN val ... would fail when "expr"
contained a GROUPING() expression or matched one of the group by
expressions, and where collatable types were involved; whereas the
supposedly identical CASE WHEN expr = val ... would succeed.

Backpatch all the way; this appears to have been wrong ever since
collations were introduced.

Per report from Guillaume Lelarge, analysis and patch by me.

Discussion: https://postgr.es/m/CAECtzeVSO_US8C2Khgfv54ZMUOBR4sWq+6_bLrETnWExHT=rFg@mail.gmail.com
Discussion: https://postgr.es/m/87muo0k0c7.fsf@news-spur.riddles.org.uk
parent feea3858
No related branches found
No related tags found
No related merge requests found
...@@ -391,11 +391,13 @@ transformDeleteStmt(ParseState *pstate, DeleteStmt *stmt) ...@@ -391,11 +391,13 @@ transformDeleteStmt(ParseState *pstate, DeleteStmt *stmt)
qry->hasSubLinks = pstate->p_hasSubLinks; qry->hasSubLinks = pstate->p_hasSubLinks;
qry->hasWindowFuncs = pstate->p_hasWindowFuncs; qry->hasWindowFuncs = pstate->p_hasWindowFuncs;
qry->hasAggs = pstate->p_hasAggs; qry->hasAggs = pstate->p_hasAggs;
if (pstate->p_hasAggs)
parseCheckAggregates(pstate, qry);
assign_query_collations(pstate, qry); assign_query_collations(pstate, qry);
/* this must be done after collations, for reliable comparison of exprs */
if (pstate->p_hasAggs)
parseCheckAggregates(pstate, qry);
return qry; return qry;
} }
...@@ -1010,8 +1012,6 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt) ...@@ -1010,8 +1012,6 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt)
qry->hasSubLinks = pstate->p_hasSubLinks; qry->hasSubLinks = pstate->p_hasSubLinks;
qry->hasWindowFuncs = pstate->p_hasWindowFuncs; qry->hasWindowFuncs = pstate->p_hasWindowFuncs;
qry->hasAggs = pstate->p_hasAggs; qry->hasAggs = pstate->p_hasAggs;
if (pstate->p_hasAggs || qry->groupClause || qry->havingQual)
parseCheckAggregates(pstate, qry);
foreach(l, stmt->lockingClause) foreach(l, stmt->lockingClause)
{ {
...@@ -1021,6 +1021,10 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt) ...@@ -1021,6 +1021,10 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt)
assign_query_collations(pstate, qry); assign_query_collations(pstate, qry);
/* this must be done after collations, for reliable comparison of exprs */
if (pstate->p_hasAggs || qry->groupClause || qry->havingQual)
parseCheckAggregates(pstate, qry);
return qry; return qry;
} }
...@@ -1470,8 +1474,6 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt) ...@@ -1470,8 +1474,6 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
qry->hasSubLinks = pstate->p_hasSubLinks; qry->hasSubLinks = pstate->p_hasSubLinks;
qry->hasWindowFuncs = pstate->p_hasWindowFuncs; qry->hasWindowFuncs = pstate->p_hasWindowFuncs;
qry->hasAggs = pstate->p_hasAggs; qry->hasAggs = pstate->p_hasAggs;
if (pstate->p_hasAggs || qry->groupClause || qry->havingQual)
parseCheckAggregates(pstate, qry);
foreach(l, lockingClause) foreach(l, lockingClause)
{ {
...@@ -1481,6 +1483,10 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt) ...@@ -1481,6 +1483,10 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
assign_query_collations(pstate, qry); assign_query_collations(pstate, qry);
/* this must be done after collations, for reliable comparison of exprs */
if (pstate->p_hasAggs || qry->groupClause || qry->havingQual)
parseCheckAggregates(pstate, qry);
return qry; return qry;
} }
......
...@@ -1655,3 +1655,22 @@ select least_agg(variadic array[q1,q2]) from int8_tbl; ...@@ -1655,3 +1655,22 @@ select least_agg(variadic array[q1,q2]) from int8_tbl;
-4567890123456789 -4567890123456789
(1 row) (1 row)
-- check collation-sensitive matching between grouping expressions
select v||'a', case v||'a' when 'aa' then 1 else 0 end, count(*)
from unnest(array['a','b']) u(v)
group by v||'a' order by 1;
?column? | case | count
----------+------+-------
aa | 1 | 1
ba | 0 | 1
(2 rows)
select v||'a', case when v||'a' = 'aa' then 1 else 0 end, count(*)
from unnest(array['a','b']) u(v)
group by v||'a' order by 1;
?column? | case | count
----------+------+-------
aa | 1 | 1
ba | 0 | 1
(2 rows)
...@@ -612,3 +612,11 @@ drop view aggordview1; ...@@ -612,3 +612,11 @@ drop view aggordview1;
-- variadic aggregates -- variadic aggregates
select least_agg(q1,q2) from int8_tbl; select least_agg(q1,q2) from int8_tbl;
select least_agg(variadic array[q1,q2]) from int8_tbl; select least_agg(variadic array[q1,q2]) from int8_tbl;
-- check collation-sensitive matching between grouping expressions
select v||'a', case v||'a' when 'aa' then 1 else 0 end, count(*)
from unnest(array['a','b']) u(v)
group by v||'a' order by 1;
select v||'a', case when v||'a' = 'aa' then 1 else 0 end, count(*)
from unnest(array['a','b']) u(v)
group by v||'a' order by 1;
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment