From 08b775f9404c33bf72313b84094357a4c45f1f61 Mon Sep 17 00:00:00 2001 From: Richard Guo Date: Wed, 11 Nov 2026 11:04:50 +0900 Subject: [PATCH v1] Fix GROUP BY column removal for NULLS NOT DISTINCT indexes remove_useless_groupby_columns() removes GROUP BY columns that are functionally dependent on other GROUP BY columns, as proven by a unique index. The index columns are normally required to be NOT NULL, but commit bd10ec529 relaxed that for indexes defined with NULLS NOT DISTINCT, on the grounds that such an index allows at most one row with a NULL in the indexed columns. That reasoning does not hold if the relation is on the nullable side of an outer join. Rows that are null-extended by the join also have NULLs in the indexed columns, so they fall into the same group as the relation's one real NULL row once the other columns are removed from the GROUP BY, even though they differ in those columns. This results in wrong query results. To fix, skip the NULLS NOT DISTINCT relaxation for relations whose GROUP BY Vars can be nulled by an outer join, and insist on NOT NULL index columns as before. That case remains safe, since no real row can then be mistaken for a null-extended one. Back-patch to v18, where the NULLS NOT DISTINCT relaxation was introduced. Author: Richard Guo Discussion: https://postgr.es/m/ Backpatch-through: 18 --- src/backend/optimizer/plan/initsplan.c | 12 ++++++++++-- src/test/regress/expected/aggregates.out | 16 ++++++++++++++++ src/test/regress/sql/aggregates.sql | 5 +++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c index 43491873ba8..b825bff22a9 100644 --- a/src/backend/optimizer/plan/initsplan.c +++ b/src/backend/optimizer/plan/initsplan.c @@ -374,6 +374,7 @@ remove_useless_groupby_columns(PlannerInfo *root) Bitmapset **groupbyattnos; List **groupbycols; Bitmapset **surplusvars; + Relids nullable_relids = NULL; bool tryremove = false; ListCell *lc; int relid; @@ -420,6 +421,10 @@ remove_useless_groupby_columns(PlannerInfo *root) relid = var->varno; Assert(relid <= list_length(parse->rtable)); + /* Remember rels whose GROUP BY Vars can be nulled by an outer join */ + if (!bms_is_empty(var->varnullingrels)) + nullable_relids = bms_add_member(nullable_relids, relid); + /* * If this isn't the first column for this relation then we now have * multiple columns. That means there might be some that can be @@ -517,9 +522,12 @@ remove_useless_groupby_columns(PlannerInfo *root) * can relax this check when the index is defined with NULLS * NOT DISTINCT as there can only be 1 NULL row, therefore * functional dependency on the unique columns is maintained, - * despite the NULL. + * despite the NULL. That doesn't hold if the rel is nullable + * by an outer join, though, as null-extended rows would be + * conflated with that NULL row. */ - if (!index->nullsnotdistinct && + if ((!index->nullsnotdistinct || + bms_is_member(relid, nullable_relids)) && !bms_is_member(indkey_attno, rel->notnullattnums)) { index_check_ok = false; diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out index ce555a31693..0845aeb5202 100644 --- a/src/test/regress/expected/aggregates.out +++ b/src/test/regress/expected/aggregates.out @@ -1711,6 +1711,22 @@ explain (costs off) select y,z from t2 group by y,z; -> Seq Scan on t2 (3 rows) +-- But not if the table is nullable by an outer join, as null-extended rows +-- would be conflated with the row having a NULL in the indexed column. +explain (costs off) +select t2.y,t2.z from t1 left join t2 on true group by t2.y,t2.z; + QUERY PLAN +---------------------------------------- + HashAggregate + Group Key: t2.y, t2.z + -> Nested Loop Left Join + -> Append + -> Seq Scan on t1 t1_1 + -> Seq Scan on t1c t1_2 + -> Materialize + -> Seq Scan on t2 +(8 rows) + drop table t1 cascade; NOTICE: drop cascades to table t1c drop table t2; diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql index 0936088b074..429f96916c5 100644 --- a/src/test/regress/sql/aggregates.sql +++ b/src/test/regress/sql/aggregates.sql @@ -600,6 +600,11 @@ alter table t2 alter column z drop not null; create unique index t2_z_uidx on t2(z) nulls not distinct; explain (costs off) select y,z from t2 group by y,z; +-- But not if the table is nullable by an outer join, as null-extended rows +-- would be conflated with the row having a NULL in the indexed column. +explain (costs off) +select t2.y,t2.z from t1 left join t2 on true group by t2.y,t2.z; + drop table t1 cascade; drop table t2; drop table t3; -- 2.37.1 (Apple Git-137.1)