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