From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andrew Gierth <andrew@tao11.riddles.org.uk>
Cc: pgsql-hackers@postgresql.org
Subject: Re: parseCheckAggregates vs. assign_query_collations
Date: Wed, 16 Jan 2019 12:49:30 -0500
Message-ID: <16969.1547660970@sss.pgh.pa.us> (raw)
In-Reply-To: <87muo0k0c7.fsf@news-spur.riddles.org.uk>
References: <87muo0k0c7.fsf@news-spur.riddles.org.uk>
Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> Looking into a bug report on the -general list about grouping sets,
> which turns out to be an issue of collation assignment: if the query has
> CASE GROUPING(expr) WHEN 1 ...
> then the expression is rejected as not matching the one in the GROUP BY
> clause, because CASE already assigned collations to the expression (as a
> special case in its transform function) while the rest of the query
> hasn't yet had them assigned, because parseCheckAggregates gets run
> before assign_query_collations.
Bleah.
> 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?
I never particularly liked assign_query_collations, as a matter of overall
system design. I'd prefer to nuke that and instead require collation
assignment to be done per-expression, ie at the end of transformExpr and
similar places. Now that we've seen this example, it's fairly clear why
collation assignment really should be considered an integral part of
expression parsing. Who's to say there aren't more gotchas of this sort
waiting to bite us? Also, if it were integrated into transformExpr as
it should have been to begin with, we would not have the need for quite
so many random calls to assign_expr_collations, with consequent bugs of
omission, cf 7a28e9aa0.
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, andrew@tao11.riddles.org.uk
Subject: Re: parseCheckAggregates vs. assign_query_collations
In-Reply-To: <16969.1547660970@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox