Re: BUG #19783: Sorted GROUPING SETS return a grouping column's value in a set that leaves it out

On Thu, 22 Oct 2026 at 19:25, PG Bug reporting form
<noreply(at)postgresql(dot)org> wrote:
> With a sorted grouping-sets plan (GroupAggregate), a column that is not
> in the current grouping set can show its real value instead of NULL,
> while GROUPING() for the same row says it is not grouped. It happens
> when the row has a NULL somewhere and the target list reads a column
> that comes after the grouping columns in the Agg's input:
>
> CREATE TABLE t (id int PRIMARY KEY, a int, b int, c int);
> INSERT INTO t VALUES (1, 10, NULL, 100);
> SET enable_hashagg = off;
> SELECT a, b, c, GROUPING(a, b) AS g
> FROM t GROUP BY GROUPING SETS ((id, a), (id, b));
>
> a | b | c | g
> ----+---+-----+---
> 10 | | 100 | 1
> 10 | | 100 | 2
> (2 rows)

Many thanks for reporting this.

What's happening here is due to the change c456e3911 made, as it now
uses a sort of slice-by-eight method of deforming the tuple's NULL
bitmap into the slot's tts_values array. When working on that, I
considered if the incremental deform should be careful to pick up
where it left off or not. I opted not, as the masking out of previous
bits or tts_isnull elements would, in most cases, cost more than it
would save.

The problem is that prepare_projection_slot() overwrites the slot's
tts_isnull elements to represent the rollup rows, but if any other
column is required from the tuple, in this case "c", then that will
overwrite the manually adjusted NULL when performing additional
deforming on that slot.

The reported case allows "c" to be in the SELECT list because it's
functionally dependent on the primary key. Ordinarily, it would be in
the GROUP BY clause or in an aggregate parameter, in which case we'd
have deformed it already. The heap tuple also needs to have at least 1
NULL value, else we wouldn't be deforming the NULL bitmask. The
deforming code would just "isnull[attnum] = false" each element as
they're deformed, and that wouldn't overwrite any previously deformed
attribute's null property.

I thought of a few ways to fix this:

1. Modify populate_isnull_array() to pick up where it left off. Likely
by adjusting the loop start byte, combined with adjusting the memcpy()
to not overwrite bytes that are previously deformed.
2. Modify ExprState code to remember the last outer varattno required,
then deform to there in prepare_projection_slot().

One problem with #2 is that if some very high varattno is needed for
the projection, but some lesser varattno is needed for the HAVING
qual, then we might deform more of the tuple than we need to when the
HAVING qual filters out the tuple. Ultimately, we'd not perform the
projection step in that case, therefore not deform the remaining
attributes. I was less concerned about that when I saw the hashed
version does this anyway. I understand it's possible to craft a case
where this might make a difference, but I suspect in practice that
it's not going to be an issue.

I found another way of doing #2 (attached) without the mods to
ExprState by reusing some existing code in nodeAgg.c which walks the
targetlist and Aggrefs looking for columns. That was only being
executed for the hashing code path, so I added some code to execute
this for the non-hash version.

David