pgsql: Fix collation assignment for aggregates with ORDER BY.
Fix collation assignment for aggregates with ORDER BY.
ORDER BY expressions were being treated the same as regular aggregate
arguments for purposes of collation determination, but really they should
not affect the aggregate's collation at all; only collations of the
aggregate's regular arguments should affect it.
In many cases this mistake would lead to incorrectly throwing a "collation
conflict" error; but in some cases the corrected code will silently assign
a different collation to the aggregate than before, for example
agg(foo ORDER BY bar COLLATE "x")
which will now use foo's collation rather than "x" for the aggregate.
Given this risk and the lack of field complaints about the issue, it
doesn't seem prudent to back-patch.
In passing, rearrange code in assign_collations_walker so that we don't
need multiple copies of the standard logic for computing collation of a
node with children. (Previously, CaseExpr duplicated the standard logic,
and we would have needed a third copy for Aggref without this change.)
Andrew Gierth and David Fetter
Branch
------
master
Details
-------
http://git.postgresql.org/pg/commitdiff/41a2760f611d1b3c1e67f755baf0a052b5cec9af
Modified Files
--------------
src/backend/parser/parse_collate.c | 177 +++++++++++++++++----------------
src/test/regress/expected/collate.out | 22 ++++
src/test/regress/sql/collate.sql | 6 +
3 files changed, 121 insertions(+), 84 deletions(-)
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
On Fri, Apr 26, 2013 at 07:49:47PM +0000, Tom Lane wrote:
Fix collation assignment for aggregates with ORDER BY.
ORDER BY expressions were being treated the same as regular aggregate
arguments for purposes of collation determination, but really they should
not affect the aggregate's collation at all; only collations of the
aggregate's regular arguments should affect it.In many cases this mistake would lead to incorrectly throwing a "collation
conflict" error; but in some cases the corrected code will silently assign
a different collation to the aggregate than before, for example
agg(foo ORDER BY bar COLLATE "x")
which will now use foo's collation rather than "x" for the aggregate.
Given this risk and the lack of field complaints about the issue, it
doesn't seem prudent to back-patch.In passing, rearrange code in assign_collations_walker so that we don't
need multiple copies of the standard logic for computing collation of a
node with children. (Previously, CaseExpr duplicated the standard logic,
and we would have needed a third copy for Aggref without this change.)Andrew Gierth and David Fetter
This needs back-patching to 9.1, where the bug was introduced.
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, 2013-04-26 at 16:46 -0700, David Fetter wrote:
On Fri, Apr 26, 2013 at 07:49:47PM +0000, Tom Lane wrote:
Given this risk and the lack of field complaints about the issue, it
doesn't seem prudent to back-patch.
...
This needs back-patching to 9.1, where the bug was introduced.
It was explained in the commit message why it was not backported.
Regards,
Jeff Davis
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Apr 26, 2013 at 05:52:03PM -0700, Jeff Davis wrote:
On Fri, 2013-04-26 at 16:46 -0700, David Fetter wrote:
On Fri, Apr 26, 2013 at 07:49:47PM +0000, Tom Lane wrote:
Given this risk and the lack of field complaints about the issue, it
doesn't seem prudent to back-patch....
This needs back-patching to 9.1, where the bug was introduced.
It was explained in the commit message why it was not backported.
Oops. Sorry about that, Tom.
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers