Re: parseCheckAggregates vs. assign_query_collations

>>>>> "Tom" == Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

>> I'll be looking into this in detail later, but right now, cam anyone
>> think of any reason why parseCheckAggregates couldn't be moved to
>> after assign_query_collations?

Tom> I never particularly liked assign_query_collations, as a matter of
Tom> overall system design. I'd prefer to nuke that and instead require
Tom> collation assignment to be done per-expression, ie at the end of
Tom> transformExpr and similar places. Now that we've seen this
Tom> example, it's fairly clear why collation assignment really should
Tom> be considered an integral part of expression parsing. Who's to say
Tom> there aren't more gotchas of this sort waiting to bite us? Also,
Tom> if it were integrated into transformExpr as it should have been to
Tom> begin with, we would not have the need for quite so many random
Tom> calls to assign_expr_collations, with consequent bugs of omission,
Tom> cf 7a28e9aa0.

Sure, this might be the right approach going forward. But right now we
need a back-patchable fix, and the above sounds a bit too intrusive for
that.

Turns out the issue can be reproduced without grouping sets too:

select case a||'' when '1' then 0 else 1 end
from (select (select random()::text) as a) s group by a||'';
ERROR: column "s.a" must appear in the GROUP BY clause or be used in an aggregate function

select case when a||'' = '1' then 0 else 1 end
from (select (select random()::text) as a) s group by a||''; -- works

--
Andrew (irc:RhodiumToad)