Re: Final Patch for GROUPING SETS - Mailing list pgsql-hackers

From Marti Raudsepp
Subject Re: Final Patch for GROUPING SETS
Date
Msg-id CABRT9RDeRjCMFnZo0c51pF-H9wT1F=EE9zwYJaKkFFLPaw6rVA@mail.gmail.com
Whole thread Raw
In response to Re: Final Patch for GROUPING SETS  (Andrew Gierth <andrew@tao11.riddles.org.uk>)
Responses Re: Final Patch for GROUPING SETS
List pgsql-hackers
On Fri, Sep 12, 2014 at 9:41 PM, Andrew Gierth
<andrew@tao11.riddles.org.uk> wrote:
> gsp1.patch         - phase 1 code patch (full syntax, limited functionality)
> gsp2.patch         - phase 2 code patch (adds full functionality using the
>                      new chained aggregate mechanism)

I gave these a try by converting my current CTE-based queries into
CUBEs and it works as expected; query time is cut in half and lines of
code is 1/4 of original. Thanks!

I only have a few trivial observations; if I'm getting too nitpicky
let me know. :)

----
Since you were asking for feedback on the EXPLAIN output on IRC, I'd
weigh in and say that having the groups on separate lines would be
significantly more readable. It took me a while to understand what's
going on in my queries due to longer table and column names and
wrapping; The comma separators between groups are hard to distinguish.
If that can be made to work with the EXPLAIN printer without too much
trouble.

So instead of:GroupAggregate  Output: four, ten, hundred, count(*)  Grouping Sets: (onek.four, onek.ten, onek.hundred),
(onek.four,
onek.ten), (onek.four), ()

Perhaps print:  Grouping Sets: (onek.four, onek.ten, onek.hundred)                 (onek.four, onek.ten)
(onek.four)                 ()
 

Or maybe:  Grouping Set: (onek.four, onek.ten, onek.hundred)  Grouping Set: (onek.four, onek.ten)  Grouping Set:
(onek.four) Grouping Set: ()
 

Both seem to work with the explain.depesz.com parser, although the 1st
won't be aligned as nicely.

----
Do you think it would be reasonable to normalize single-set grouping
sets into a normal GROUP BY? Such queries would be capable of using
HashAggregate, but the current code doesn't allow that. For example:

set enable_sort=off;
explain select two, count(*) from onek group by grouping sets (two);
Could be equivalent to:
explain select two, count(*) from onek group by two;

----
I'd expect GROUP BY () to be fully equivalent to having no GROUP BY
clause, but there's a difference in explain output. The former
displays "Grouping Sets: ()" which is odd, since none of the grouping
set keywords were used.

# explain select count(*) from onek group by ();Aggregate  (cost=77.78..77.79 rows=1 width=0)  Grouping Sets: ()  ->
IndexOnly Scan using onek_stringu1 on onek  (cost=0.28..75.28
 
rows=1000 width=0)

Regards,
Marti



pgsql-hackers by date:

Previous
From: Peter Geoghegan
Date:
Subject: Re: Collations and Replication; Next Steps
Next
From: Kouhei Kaigai
Date:
Subject: Re: [v9.5] Custom Plan API