Horton

Re: Combination of ordered-set aggregate function terminates JDBC connection on PostgreSQL 9.6.5

Tom Lane <tgl@sss.pgh.pa.us>

View thread
From: Tom Lane <tgl@sss.pgh.pa.us>
To: David Rowley <david.rowley@2ndquadrant.com>
Cc: Lukas Eder <lukas.eder@gmail.com>, PostgreSQL mailing lists <pgsql-bugs@postgresql.org>, Heikki Linnakangas <hlinnaka@iki.fi>
Date: 2017-10-12T02:27:05Z
Lists: pgsql-bugs

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Treat aggregate direct arguments as per-agg data not per-trans data.

  2. Allow the built-in ordered-set aggregates to share transition state.

  3. Fix AggGetAggref() so it won't lie to aggregate final functions.

  4. Prevent sharing transition states between ordered-set aggregates.

David Rowley <david.rowley@2ndquadrant.com> writes:
> On 12 October 2017 at 08:51, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> However, in order to make use of tuplesort_rescan, we'd have had
>> to pass randomAccess = true to tuplesort_begin_xxx, which would
>> be rather an annoying overhead for the majority case where there
>> isn't a potential for reuse.

> Yeah maybe if core tracked the total number of references in
> AggStatePerTrans, and finalize_aggregate() incremented another counter
> to track how many times the final function had been called on this
> state, then if there was some way to expose that information to the
> final function, it would know if it was the first or the last final
> function to use the state.

That seems kind of irrelevant, at least for the existing OSAs.
To know what value of randomAccess to pass to the tuplesort setup,
we have to know *at the first transition-function call* whether
there may be multiple final-function calls coming up.  So what
what I'm imagining is a simple boolean result "yes, there will be
only one finalfn call, so it can destructively modify the transition
state", or "there might be more than one finalfn call, so the finalfn(s)
must preserve transition state".  And this info has to be available
throughout the aggregate run.

>> This might be a bigger change than we want to push into the back
>> branches.  In that case probably a back-patchable fix is to hack
>> nodeAgg.c so it will never combine input states for OSAs.

> I've attached a patch which does this.

Needs to reject plain OSAs too, not just hypotheticals.  Pushed
with that fix and some test cases.

			regards, tom lane