Re: Wrong results with grouping sets

On Mon, Sep 25, 2023 at 3:11 PM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:

> If the grouping expression is a Var or PHV, we can just set its
> nullingrels, very straightforward. For an expression that is neither a
> Var nor a PHV, I'm not quite sure how to set the nullingrels. I tried
> the idea of wrapping it in a new PHV to carry the nullingrels, but that
> may cause some unnecessary plan diffs. In the patch for such an
> expression I just set the nullingrels of Vars or PHVs that are contained
> in it. This is not really 'correct' in theory, because it is the whole
> expression that can be nullable by grouping sets, not its individual
> vars. But it works in practice, because what we need is that the
> expression can be somehow distinguished from the same expression in ECs,
> and marking its vars is sufficient for this purpose. But what if the
> expression is variable-free? This is the point that needs more work.
> Fow now the patch just handles variable-free expressions of type Const,
> by wrapping it in a new PHV.
>

For a variable-free expression, if it contains volatile functions, SRFs,
aggregates, or window functions, it would not be treated as a member of
EC that is redundant (see get_eclass_for_sort_expr()). That means it
would not be removed from the pathkeys list, so we do not need to set
the nullingrels for it. Otherwise we can just wrap the expression in a
PlaceHolderVar. Attached is an updated patch to do that.

BTW, this wrong results issue has existed ever since grouping sets was
introduced in v9.5, and there were field reports complaining about this
issue. I think it would be great if we can get rid of it. I'm still
not sure what we should do in back branches. But let's fix it at least
on v16 and later.

Thanks
Richard