pgsql: Prevent sharing transition states between ordered-set aggregates

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-committers(at)postgresql(dot)org
Subject: pgsql: Prevent sharing transition states between ordered-set aggregates
Date: 2017-10-12 02:18:30
Message-ID: E1e2T4w-0004mg-4f@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Prevent sharing transition states between ordered-set aggregates.

This ought to work, but the built-in OSAs are not capable of coping,
because their final-functions destructively modify their transition
state (specifically, the contained tuplesort object). That was fine
when those functions were written, but commit 804163bc2 moved the
goalposts without telling orderedsetaggs.c.

We should fix the built-in OSAs to support this, but it will take
a little work, especially if we don't want to sacrifice performance
in the normal non-shared-state case. Given that it took a year after
9.6 release for anyone to notice this bug, we should not prioritize
sharable-state over nonsharable-state performance. And a proper fix
is likely to be more complicated than we'd want to back-patch, too.

Therefore, let's just put in this stop-gap patch to prevent nodeAgg.c
from choosing to use shared state for OSAs. We can revert it in HEAD
when we get a better fix.

Report from Lukas Eder, diagnosis by me, patch by David Rowley.
Back-patch to 9.6 where the problem was introduced.

Discussion: https://postgr.es/m/CAB4ELO5RZhOamuT9Xsf72ozbenDLLXZKSk07FiSVsuJNZB861A@mail.gmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/52328727bea4d9f95af9622e4624b9d1492df88e

Modified Files
--------------
src/backend/executor/nodeAgg.c | 10 ++++++++++
src/test/regress/expected/aggregates.out | 19 +++++++++++++++++++
src/test/regress/sql/aggregates.sql | 11 +++++++++++
3 files changed, 40 insertions(+)

Browse pgsql-committers by date

  From Date Subject
Next Message Andres Freund 2017-10-12 04:01:42 pgsql: Replace remaining uses of pq_sendint with pq_sendint{8, 16, 32}.
Previous Message Andres Freund 2017-10-12 02:11:02 pgsql: Temporary attempt at a workaround for further MSVC restrict buil