Re: [HACKERS] Hash support for grouping sets

> "Andres" == Andres Freund writes: Andres> a) cast result of lfirst/lnext/whatnot. Again, what we need here is something like #define lfirst_node(_type_, l) (castNode(_type_, lfirst(l))) etc. -- Andrew (irc:RhodiumToad) -- Sent via pgsql-hackers mailing list

Re: [HACKERS] Hash support for grouping sets

> "Andres" == Andres Freund writes: Andres> We usually cast the result of palloc. >> Rough count in the backend has ~400 without casts to ~1350 with, so >> this doesn't seem to have been consistently enforced. Andres> Yea, but we're still trying. Well, a lot of the

Re: [HACKERS] Hash support for grouping sets

> "Mark" == Mark Dilger writes: Mark> Is there a performance test case where this patch should shine Mark> brightest? I'd like to load a schema with lots of data, and run Mark> a grouping sets query, both before and after applying the patch, Mark> to see what

Re: [HACKERS] Hash support for grouping sets

On 2017-03-23 05:09:46 +, Andrew Gierth wrote: > > "Andres" == Andres Freund writes: > > >> - Assert(newphase == 0 || newphase == aggstate->current_phase + 1); > >> + Assert(newphase <= 1 || newphase == aggstate->current_phase + 1); > > Andres> I think this

Re: [HACKERS] Hash support for grouping sets

> On Mar 23, 2017, at 11:22 AM, Andrew Gierth > wrote: > >> "Mark" == Mark Dilger writes: > > Mark> You define DiscreteKnapsack to take integer weights and double > Mark> values, and perform the usual Dynamic Programming algorithm to

Re: [HACKERS] Hash support for grouping sets

On 2017-03-23 03:43:57 +, Andrew Gierth wrote: > > "Andres" == Andres Freund writes: > > Andres> Changes to advance_aggregates() are, in my experience, quite > Andres> likely to have performance effects. This needs some > Andres> performance tests. > [...] >

Re: [HACKERS] Hash support for grouping sets

> "Mark" == Mark Dilger writes: Mark> You define DiscreteKnapsack to take integer weights and double Mark> values, and perform the usual Dynamic Programming algorithm to Mark> solve. But the only place you call this, you pass in NULL for Mark> the values,

Re: [HACKERS] Hash support for grouping sets

> "Andres" == Andres Freund writes: >> - Assert(newphase == 0 || newphase == aggstate->current_phase + 1); >> + Assert(newphase <= 1 || newphase == aggstate->current_phase + 1); Andres> I think this somewhere in the file header needs an expanded Andres>

Re: [HACKERS] Hash support for grouping sets

> On Mar 22, 2017, at 8:09 AM, Mark Dilger wrote: > > >> On Mar 22, 2017, at 7:55 AM, Andrew Gierth >> wrote: >> >> [snip] >> >> This thread seems to have gone quiet - is it time for me to just go >> ahead and commit the thing anyway?

Re: [HACKERS] Hash support for grouping sets

> "Andres" == Andres Freund writes: Andres> Changes to advance_aggregates() are, in my experience, quite Andres> likely to have performance effects. This needs some Andres> performance tests. [...] Andres> Looks like it could all be noise, but it seems worthwhile to

Re: [HACKERS] Hash support for grouping sets

Hi, > +/* > + * Switch to phase "newphase", which must either be 0 or 1 (to reset) or > * current_phase + 1. Juggle the tuplesorts accordingly. > */ > static void > initialize_phase(AggState *aggstate, int newphase) > { > - Assert(newphase == 0 || newphase == aggstate->current_phase +

Re: [HACKERS] Hash support for grouping sets

[snip] This thread seems to have gone quiet - is it time for me to just go ahead and commit the thing anyway? Anyone else want to weigh in? -- Andrew (irc:RhodiumToad) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

Re: [HACKERS] Hash support for grouping sets

> "Mark" == Mark Dilger writes: Mark> Hi Andrew, Mark> Reviewing the patch a bit more, I find it hard to understand the Mark> comment about passing -1 as a flag for finalize_aggregates. Any Mark> chance you can spend a bit more time word-smithing that code

Re: [HACKERS] Hash support for grouping sets

> On Mar 8, 2017, at 9:40 AM, Andrew Gierth wrote: > >> "Mark" == Mark Dilger writes: > > Mark> Hi Andrew, > > Mark> Reviewing the patch a bit more, I find it hard to understand the > Mark> comment about passing -1 as a flag for

Re: [HACKERS] Hash support for grouping sets

> "Mark" == Mark Dilger writes: Mark> Hi Andrew, Mark> Reviewing the patch a bit more, I find it hard to understand the Mark> comment about passing -1 as a flag for finalize_aggregates. Any Mark> chance you can spend a bit more time word-smithing that code

Re: [HACKERS] Hash support for grouping sets

Hi Andrew, Reviewing the patch a bit more, I find it hard to understand the comment about passing -1 as a flag for finalize_aggregates. Any chance you can spend a bit more time word-smithing that code comment? @@ -1559,7 +1647,9 @@ prepare_projection_slot(AggState *aggstate, TupleTableSlot

Re: [HACKERS] Hash support for grouping sets

> On Mar 8, 2017, at 5:47 AM, Andrew Gierth wrote: > >> "Mark" == Mark Dilger writes: > > Mark> On my MacBook, `make check-world` gives differences in the > Mark> contrib modules: > > Thanks! Latest cleaned up version of patch is

Re: [HACKERS] Hash support for grouping sets

> On Mar 8, 2017, at 2:30 AM, Andrew Gierth wrote: > >> "Mark" == Mark Dilger writes: > > Mark> On linux/gcc the patch generates a warning in nodeAgg.c that is > Mark> fairly easy to fix. Using -Werror to make catching the error >

Re: [HACKERS] Hash support for grouping sets

> "Mark" == Mark Dilger writes: Mark> On linux/gcc the patch generates a warning in nodeAgg.c that is Mark> fairly easy to fix. Using -Werror to make catching the error Mark> easier, I get: what gcc version is this exactly? -- Andrew (irc:RhodiumToad) --

Re: [HACKERS] Hash support for grouping sets

The following review has been posted through the commitfest application: make installcheck-world: tested, failed Implements feature: not tested Spec compliant: not tested Documentation:not tested On linux/gcc the patch generates a warning in nodeAgg.c that is fairly

Re: [HACKERS] Hash support for grouping sets

> On Mar 7, 2017, at 1:43 PM, Mark Dilger wrote: > > On my MacBook, `make check-world` gives differences in the contrib modules: I get the same (or similar -- didn't check) regression failure on CentOS, so this doesn't appear to be MacBook / hardware specific. Mark

Re: [HACKERS] Hash support for grouping sets

The following review has been posted through the commitfest application: make installcheck-world: tested, failed Implements feature: not tested Spec compliant: not tested Documentation:not tested On my MacBook, `make check-world` gives differences in the contrib

Re: [HACKERS] Hash support for grouping sets

> "Thom" == Thom Brown writes: Thom> This doesn't apply cleanly to latest master. Could you please Thom> post a rebased patch? Sure. -- Andrew (irc:RhodiumToad) diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index c9e0a3e..480a07e 100644

Re: [HACKERS] Hash support for grouping sets

On 6 January 2017 at 03:48, Andrew Gierth wrote: > Herewith a patch for doing grouping sets via hashing or mixed hashing > and sorting. > > The principal objective is to pick whatever combination of grouping sets > has an estimated size that fits in work_mem, and

Re: [HACKERS] Hash support for grouping sets

On Mon, Jan 16, 2017 at 10:59 AM, Finnerty, Jim wrote: > The ability to exploit hashed aggregation within sorted groups, when the > order of the input stream can be exploited this way, is potentially a useful > way to improve aggregation performance more generally. This

Re: [HACKERS] Hash support for grouping sets

The ability to exploit hashed aggregation within sorted groups, when the order of the input stream can be exploited this way, is potentially a useful way to improve aggregation performance more generally. This would potentially be beneficial when the input size is expected to be larger than

Re: [HACKERS] Hash support for grouping sets

On Thu, Jan 5, 2017 at 10:48 PM, Andrew Gierth wrote: > Herewith a patch for doing grouping sets via hashing or mixed hashing > and sorting. Cool. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via

[HACKERS] Hash support for grouping sets

Herewith a patch for doing grouping sets via hashing or mixed hashing and sorting. The principal objective is to pick whatever combination of grouping sets has an estimated size that fits in work_mem, and minimizes the number of sorting passes we need to do over the data, and hash those. (Yes,