Thread: Bug Fix: COLLATE with multiple ORDER BYs in aggregates
Folks, While testing the upcoming FILTER clause for aggregates, Erik Rijkers uncovered a long-standing bug in $subject, namely that this case wasn't handled. Please find attached a patch by Andrew Gierth and myself which fixes this issue and adds a regression test to ensure it remains fixed. Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Attachment
On Tue, Apr 23, 2013 at 09:57:27AM -0700, David Fetter wrote:
> Folks,
>
> While testing the upcoming FILTER clause for aggregates, Erik Rijkers
> uncovered a long-standing bug in $subject, namely that this case
> wasn't handled. Please find attached a patch by Andrew Gierth and
> myself which fixes this issue and adds a regression test to ensure it
> remains fixed.
Please see below results when I run the regression test query on git
master, REL_9_2_STABLE, and REL9_1_STABLE, respectively:
$ psql postgres
psql (9.3devel)
Type "help" for help.
shackle@postgres:5493=# SELECT array_agg(a COLLATE "C" ORDER BY b COLLATE "POSIX") FROM (VALUES ('foo','bar')) v(a,b);
ERROR: collation mismatch between explicit collations "C" and "POSIX"
LINE 1: SELECT array_agg(a COLLATE "C" ORDER BY b COLLATE "POSIX") F...
^
$ psql postgres
psql (9.2.4)
Type "help" for help.
shackle@postgres:5492=# SELECT array_agg(a COLLATE "C" ORDER BY b COLLATE "POSIX") FROM (VALUES ('foo','bar')) v(a,b);
ERROR: collation mismatch between explicit collations "C" and "POSIX"
LINE 1: SELECT array_agg(a COLLATE "C" ORDER BY b COLLATE "POSIX") F...
^
$ psql postgres
psql (9.1.9)
Type "help" for help.
shackle@postgres:5491=# SELECT array_agg(a COLLATE "C" ORDER BY b COLLATE "POSIX") FROM (VALUES ('foo','bar')) v(a,b);
ERROR: collation mismatch between explicit collations "C" and "POSIX"
LINE 1: SELECT array_agg(a COLLATE "C" ORDER BY b COLLATE "POSIX") F...
^
Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
David Fetter <david@fetter.org> writes:
> While testing the upcoming FILTER clause for aggregates, Erik Rijkers
> uncovered a long-standing bug in $subject, namely that this case
> wasn't handled. Please find attached a patch by Andrew Gierth and
> myself which fixes this issue and adds a regression test to ensure it
> remains fixed.
I don't find this patch to be a good idea.
The argument for it seems to be that
array_agg(a COLLATE "C" ORDER BY b COLLATE "POSIX")
should not throw an error, but why not? And what does that have to do
with whacking around the code for CASE?
regards, tom lane
On 2013-04-25 13:42:32 -0400, Tom Lane wrote: > David Fetter <david@fetter.org> writes: > > While testing the upcoming FILTER clause for aggregates, Erik Rijkers > > uncovered a long-standing bug in $subject, namely that this case > > wasn't handled. Please find attached a patch by Andrew Gierth and > > myself which fixes this issue and adds a regression test to ensure it > > remains fixed. > > I don't find this patch to be a good idea. > > The argument for it seems to be that > > array_agg(a COLLATE "C" ORDER BY b COLLATE "POSIX") > > should not throw an error, but why not? Uh. Why should it? SELECT foo COLLATE "C" FROM ... ORDER BY bar COLLATE "POSIX" doesn't throw one either? > And what does that have to do with whacking around the code for CASE? I guess that's to avoid to repeat that already triplicated block of code once more. The goal seems to make sense to me, although I am not 100% that thats the nicest solution to get of the repetition. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes:
> On 2013-04-25 13:42:32 -0400, Tom Lane wrote:
>> The argument for it seems to be that
>> array_agg(a COLLATE "C" ORDER BY b COLLATE "POSIX")
>> should not throw an error, but why not?
> Uh. Why should it? SELECT foo COLLATE "C" FROM ... ORDER BY bar COLLATE
> "POSIX" doesn't throw one either?
After thinking about it a bit more, this case *should* throw an error:
string_agg(a COLLATE "C", b COLLATE "POSIX")
but these should not:
array_agg(a COLLATE "C" ORDER BY b COLLATE "POSIX")
array_agg(a ORDER BY b COLLATE "C", c COLLATE "POSIX")
that is, the ORDER BY expression(s) ought to be considered independently
rather than as part of the agg's argument list.
It looks like the proposed patch gets this right, but the proposed
test cases really fail to illuminate the problem IMO.
regards, tom lane
On Thu, Apr 25, 2013 at 06:04:10PM -0400, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > On 2013-04-25 13:42:32 -0400, Tom Lane wrote: > >> The argument for it seems to be that > >> array_agg(a COLLATE "C" ORDER BY b COLLATE "POSIX") > >> should not throw an error, but why not? > > > Uh. Why should it? SELECT foo COLLATE "C" FROM ... ORDER BY bar COLLATE > > "POSIX" doesn't throw one either? > > After thinking about it a bit more, this case *should* throw an error: > > string_agg(a COLLATE "C", b COLLATE "POSIX") > > but these should not: > > array_agg(a COLLATE "C" ORDER BY b COLLATE "POSIX") > > array_agg(a ORDER BY b COLLATE "C", c COLLATE "POSIX") > > that is, the ORDER BY expression(s) ought to be considered independently > rather than as part of the agg's argument list. > > It looks like the proposed patch gets this right, but the proposed > test cases really fail to illuminate the problem IMO. > > regards, tom lane Am I understanding correctly that you want the code left alone and the test case expanded as above? Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
David Fetter <david@fetter.org> writes:
> While testing the upcoming FILTER clause for aggregates, Erik Rijkers
> uncovered a long-standing bug in $subject, namely that this case
> wasn't handled. Please find attached a patch by Andrew Gierth and
> myself which fixes this issue and adds a regression test to ensure it
> remains fixed.
Applied with minor editorialization.
regards, tom lane