Re: Final Patch for GROUPING SETS

On 2015-05-12 05:36:19 +0200, Andres Freund wrote:
> > Another controversial item was the introduction of GroupedVar. The need
> > for this can be avoided by explicitly setting to NULL the relevant
> > columns of the representative group tuple when evaluating result rows,
> > but (a) I don't think that's an especially clean approach (though I'm
> > not pushing back very hard on it) and (b) the logic needed in its
> > absence is different between the current chaining implementation and a
> > possible union implementation, so I decided against making any changes
> > on wasted-effort grounds.
>
> Seems like fairly minor point to me. I very tentatively lean towards
> setting the columns in the group tuple to NULL.

I'm pretty sure by now that I dislike the introduction of GroupedVar,
and not just tentatively. While I can see why you found its
introduction to be nicer than fiddling with the result tuple, for me the
disadvantages seem to outweigh the advantage. For one it's rather wierd
to have Var nodes be changed into GroupedVar in setrefs.c. The number
of places that need to be touched even when it's a 'planned stmt only'
type of node is still pretty large.

Andrew: I'll work on changing this in a couple hours unless you're
speaking up about doing it yourself.

Greetings,

Andres Freund