From: Andrew Gierth <andrew@tao11.riddles.org.uk>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org
Subject: Re: parseCheckAggregates vs. assign_query_collations
In-Reply-To: <16969.1547660970@sss.pgh.pa.us> (Tom Lane's message of "Wed, 16
	Jan 2019 12:49:30 -0500")
Message-ID: <871s5cj9mn.fsf@news-spur.riddles.org.uk>
References: <87muo0k0c7.fsf@news-spur.riddles.org.uk>
	<16969.1547660970@sss.pgh.pa.us>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (berkeley-unix)
Date: Thu, 17 Jan 2019 02:48:31 +0000
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

>>>>> "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)