Horton

Re: An oversight in ExecInitAgg for grouping sets

Richard Guo <guofenglinux@gmail.com>

View thread View on postgr.es
From: Richard Guo <guofenglinux@gmail.com>
To: PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2022-12-27T07:12:17Z
Lists: pgsql-hackers

Attachments

On Thu, Dec 22, 2022 at 2:02 PM Richard Guo <guofenglinux@gmail.com> wrote:

> I happened to notice $subject.  It happens when we build eqfunctions for
> each grouping set.
>
>      /* for each grouping set */
>      for (int k = 0; k < phasedata->numsets; k++)
>      {
>          int         length = phasedata->gset_lengths[k];
>
>          if (phasedata->eqfunctions[length - 1] != NULL)
>              continue;
>
>          phasedata->eqfunctions[length - 1] =
>              execTuplesMatchPrepare(scanDesc,
>                                     length,
>                                     aggnode->grpColIdx,
>                                     aggnode->grpOperators,
>                                     aggnode->grpCollations,
>                                     (PlanState *) aggstate);
>      }
>
> If it is an empty grouping set, its length will be zero, and accessing
> phasedata->eqfunctions[length - 1] is not right.
>
> I think we can just skip building the eqfunctions for empty grouping
> set.
>

Attached is a trivial patch for the fix.

Thanks
Richard

Commits

  1. Adjust Valgrind macro usage to protect chunk headers

  2. Fix typo in memutils_memorychunk.h

  3. Avoid reference to nonexistent array element in ExecInitAgg().