Re: Final Patch for GROUPING SETS

From: Andres Freund <andres(at)anarazel(dot)de>
To: Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Greg Stark <stark(at)mit(dot)edu>, Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Josh Berkus <josh(at)agliodbs(dot)com>, Greg Sabino Mullane <greg(at)turnstep(dot)com>, Marti Raudsepp <marti(at)juffo(dot)org>, Noah Misch <noah(at)leadboat(dot)com>, Tomas Vondra <tv(at)fuzzy(dot)cz>, Atri Sharma <atri(dot)jiit(at)gmail(dot)com>, Svenne Krap <svenne(at)krap(dot)dk>
Subject: Re: Final Patch for GROUPING SETS
Date: 2015-05-12 23:48:41
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 2015-05-12 20:40:49 +0200, Andres Freund wrote:
> On 2015-05-12 05:36:19 +0200, Andres Freund wrote:
> > What I dislike so far:
> > * Minor formatting things. Just going to fix and push the ones I
> > dislike.
> > * The Hopcroft-Karp stuff not being separate
> > * The increased complexity of grouping_planner. It'd imo be good if some
> > of that could be refactored into a separate function. Specifically the
> > else if (parse->hasAggs || (parse->groupingSets && parse->groupClause))
> > block.
> > * I think it'd not hurt to add rule deparse check for the function in
> > GROUPING SETS case. I didn't see one at least.
> * The code in nodeAgg.c isn't pretty in places. Stuff like if
> (node->chain_depth > 0) estate->agg_chain_head = save_chain_head;...
> Feels like a good bit of cleanup would be possible there.

In the executor I'd further like:
* to split agg_retrieve_direct into a version for grouping sets and one
without. I think that'll be a pretty clear win for clarity.
* to spin out common code between agg_retrieve_direct (in both the
functions its split into), agg_retrieve_hashed and
agg_retrieve_chained. It should e.g. be fairly simple to spin out the
tail end processing of a input group (finalize_aggregate loop,
ExecQual) into a separate function.

Andrew, are you going to be working on any of these?


Andres Freund

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Kouhei Kaigai 2015-05-13 01:00:21 Re: Custom/Foreign-Join-APIs (Re: [v9.5] Custom Plan API)
Previous Message Peter Geoghegan 2015-05-12 23:23:54 Re: Minor ON CONFLICT related fixes