BUG #19752: GROUP BY on a constant and a cast to a length-limited array type crashes the server while planning

PG Bug reporting form <noreply@postgresql.org> writes:

I found a planner crash (SIGSEGV) in PostgreSQL 18 with the following
query:

SELECT ARRAY[]::varchar(500)[]
GROUP BY CAST('true' AS boolean);

Cause
For ARRAY[]::varchar(500)[], the parser builds an ArrayCoerceExpr whose
elemexpr is a per-element template:
varchar(CaseTestExpr, 500, true)
where the final "true" is the internal isExplicit flag.
substitute_grouped_columns_mutator() in parse_agg.c replaces any
subexpression that equal()s a GROUP BY expression with a Var referencing
the RTE_GROUP RTE. It also descends into elemexpr. The GROUP BY
expression is Const(bool, true), and the internal isExplicit flag is also
Const(bool, true), so the flag is wrongly replaced by a Var. (Dumping
elemexpr at the point of failure on 18.4 shows the third argument is a
VAR rather than a CONST.)

Yeah, that's nasty, and what's nastier is that even non-crash cases
might result in seriously wrong answers. We shouldn't be replacing
constant subexpressions by group Vars ever. I'm inclined to think
that GROUP BY items should not be candidates for this
search-and-replace unless they contain at least one level-zero Var.
But maybe that's too simplistic?

regards, tom lane

пт, 9 окт. 2026 г. в 00:32, Tom Lane <tgl@sss.pgh.pa.us>:

PG Bug reporting form <noreply@postgresql.org> writes:

I found a planner crash (SIGSEGV) in PostgreSQL 18 with the following
query:

SELECT ARRAY[]::varchar(500)[]
GROUP BY CAST('true' AS boolean);

Cause
For ARRAY[]::varchar(500)[], the parser builds an ArrayCoerceExpr whose
elemexpr is a per-element template:
varchar(CaseTestExpr, 500, true)
where the final "true" is the internal isExplicit flag.
substitute_grouped_columns_mutator() in parse_agg.c replaces any
subexpression that equal()s a GROUP BY expression with a Var referencing
the RTE_GROUP RTE. It also descends into elemexpr. The GROUP BY
expression is Const(bool, true), and the internal isExplicit flag is also
Const(bool, true), so the flag is wrongly replaced by a Var. (Dumping
elemexpr at the point of failure on 18.4 shows the third argument is a
VAR rather than a CONST.)

Yeah, that's nasty, and what's nastier is that even non-crash cases
might result in seriously wrong answers. We shouldn't be replacing
constant subexpressions by group Vars ever. I'm inclined to think
that GROUP BY items should not be candidates for this
search-and-replace unless they contain at least one level-zero Var.
But maybe that's too simplistic?

regards, tom lane

Hi, Tom!

If I analyzed the code correctly, the conclusions are as follows:

We should still replace the grouped constant itself. Bisect puts the
crash at f5050f795ae ("Mark expressions nullable by grouping sets"),
whose parent 247dea89f76 does not crash. 247dea89f76 returns on Const
before equal(). f5050f795ae moves that return to after equal(), and
the comment says why. A GROUP BY constant is no longer constant after
the grouping step. The regress query added in that commit is

select 1 as one group by rollup(one) order by one nulls first;

and the expected result is a null row followed by 1.

Skipping every GROUP BY item that contains no level-zero Var is too
broad. The item in that query is a Const, so it contains no Var, and
the commit exists to keep it a candidate. The constants that must not
be replaced are the other Const nodes that happen to compare equal.

--
Regards,
Rachitskiy Andrey

Andrey Rachitskiy <pl0h0yp1@gmail.com> writes:

пт, 9 окт. 2026 г. в 00:32, Tom Lane <tgl@sss.pgh.pa.us>:

Yeah, that's nasty, and what's nastier is that even non-crash cases
might result in seriously wrong answers. We shouldn't be replacing
constant subexpressions by group Vars ever. I'm inclined to think
that GROUP BY items should not be candidates for this
search-and-replace unless they contain at least one level-zero Var.
But maybe that's too simplistic?

We should still replace the grouped constant itself. Bisect puts the
crash at f5050f795ae ("Mark expressions nullable by grouping sets"),
whose parent 247dea89f76 does not crash. 247dea89f76 returns on Const
before equal(). f5050f795ae moves that return to after equal(), and
the comment says why. A GROUP BY constant is no longer constant after
the grouping step. The regress query added in that commit is

select 1 as one group by rollup(one) order by one nulls first;

I think that's different: the "group by one" should identify the
first TLE and nothing else. Otherwise we have insanities like

select 1 as one, 1+2 as three group by rollup(one)

affecting both columns. (I see that it does not, right now,
but it's hard to argue why not if you think replacing constants
is okay.)

Skipping every GROUP BY item that contains no level-zero Var is too
broad. The item in that query is a Const, so it contains no Var, and
the commit exists to keep it a candidate. The constants that must not
be replaced are the other Const nodes that happen to compare equal.

We might need to special-case TLEs that are identified this way.
The basic problem is how do you tell which Consts are okay to replace,
and what I'm saying is "none of them, except the identified TLE".

regards, tom lane

пт, 9 окт. 2026 г. в 01:10, Tom Lane <tgl@sss.pgh.pa.us>:

Skipping every GROUP BY item that contains no level-zero Var is too
broad. The item in that query is a Const, so it contains no Var, and
the commit exists to keep it a candidate. The constants that must not
be replaced are the other Const nodes that happen to compare equal.

We might need to special-case TLEs that are identified this way.
The basic problem is how do you tell which Consts are okay to replace,
and what I'm saying is "none of them, except the identified TLE".

Agreed.

None of the Consts should be replaced, except the targetlist
entry that GROUP BY identified.

That entry already carries a ressortgroupref. When its expression is a
Const or Param, substitute_grouped_columns_mutator() replaces the entry
as a whole. The match is on ressortgroupref, because groupClauses may
be a copy after join alias flattening. These items are skipped in the
equal() loop, so the same value elsewhere stays a constant. Param
takes the same path as Const, since the existing code accepts the two
together.

select 1 as one group by rollup(one) order by one nulls first still
returns a null row and then 1.
select 1 as one, 1+2 as three group by rollup(one) leaves three as 3.

The reported query no longer crashes.

GROUP BY true identified the boolean constant, and the true inside
ArrayCoerceExpr.elemexpr was a different Const.

Patch attached.

--
Regards,
Rachitskiy Andrey

пн, 12 окт. 2026 г. в 22:48, Mert Kul <mertkul669@gmail.com>:

Thanks for working on this!

Is this patch considered the definitive fix for the underlying issue, or
is it more of a temporary workaround?
In particular, do you think matching grouping expressions by *ressortgroupref
*fully addresses the problem,
including the potential for incorrect query results, or are further
changes needed?

Hi, Mert!

We need Tom to look at my solution, he might not like it :). Although I
think I took his wishes into account.

--
Regards,
Rachitskiy Andrey