Re: parseCheckAggregates vs. assign_query_collations

>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.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)