Skip to content

Fix ORCA GbAgg drop for nullable UNIQUE keys #1941

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
roseduan wants to merge 1 commit into
base: main
Choose a base branch
from
+168 −2
Open

Fix ORCA GbAgg drop for nullable UNIQUE keys #1941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/backend/gporca/libgpopt/src/xforms/CXformSimplifyGbAgg.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ CXformSimplifyGbAgg::FDropGbAgg(CMemoryPool *mp, CExpression *pexpr,
return false;
}

// A UNIQUE key permits multiple NULLs (NULL is never "equal" for uniqueness
// checks), but DISTINCT/GROUP BY must still collapse them into one row.
// Only drop the GbAgg when every key column is also provably NOT NULL.
CColRefSet *pcrsNotNull = pexprRelational->DeriveNotNullColumns();
const ULONG ulKeys = pkc->Keys();
BOOL fDrop = false;
for (ULONG ul = 0; !fDrop && ul < ulKeys; ul++)
Expand All @@ -111,7 +115,8 @@ CXformSimplifyGbAgg::FDropGbAgg(CMemoryPool *mp, CExpression *pexpr,

CColRefSet *pcrsGrpCols = GPOS_NEW(mp) CColRefSet(mp);
pcrsGrpCols->Include(popAgg->Pdrgpcr());
BOOL fGrpColsHasKey = pcrsGrpCols->ContainsAll(pcrs);
BOOL fGrpColsHasKey =
pcrsGrpCols->ContainsAll(pcrs) && pcrsNotNull->ContainsAll(pcrs);

pcrs->Release();
pcrsGrpCols->Release();
Expand Down
69 changes: 69 additions & 0 deletions src/test/regress/expected/bfv_aggregate.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2172,6 +2172,75 @@ commit;
ERROR: duplicate key value violates unique constraint "t3_pkey" (seg1 127.0.0.1:7003 pid=86457)
DETAIL: Key (a, b)=(1, 1) already exists.
drop table t1, t2, t3, t4, t5, t6;
-- Test NULLs in a UNIQUE key column. A UNIQUE constraint allows multiple NULLs
-- (NULL is never "equal" for uniqueness checks), but grouping columns that
-- cover such a key must still collapse those NULLs into a single row, so
-- dropping the GbAgg is only safe when every key column is also NOT NULL.
drop table if exists t7, t8, t9;
NOTICE: table "t7" does not exist, skipping
NOTICE: table "t8" does not exist, skipping
NOTICE: table "t9" does not exist, skipping
create table t7 (a int unique, b int);
create table t8 (a int unique not null, b int);
create table t9 (a int primary key, b int);
insert into t7 values (1, 1), (2, 2), (null, 3), (null, 4), (null, 5);
insert into t8 values (1, 1), (2, 2), (3, 3);
insert into t9 values (1, 1), (2, 2), (3, 3);
explain (costs off) select distinct a from t7;

@yjhjstz yjhjstz Sep 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try test on partitioned table ?

QUERY PLAN
------------------------------------------
Gather Motion 3:1 (slice1; segments: 3)
-> HashAggregate
Group Key: a
-> Seq Scan on t7
Optimizer: Postgres query optimizer
(5 rows)

select distinct a from t7 order by a;
a
---
1
2

(3 rows)

explain (costs off) select distinct a from t8;
QUERY PLAN
------------------------------------------
Gather Motion 3:1 (slice1; segments: 3)
-> HashAggregate
Group Key: a
-> Seq Scan on t8
Optimizer: Postgres query optimizer
(5 rows)

select distinct a from t8 order by a;
a
---
1
2
3
(3 rows)

explain (costs off) select distinct a from t9;
QUERY PLAN
------------------------------------------
Gather Motion 3:1 (slice1; segments: 3)
-> HashAggregate
Group Key: a
-> Seq Scan on t9
Optimizer: Postgres query optimizer
(5 rows)

select distinct a from t9 order by a;
a
---
1
2
3
(3 rows)

drop table t7, t8, t9;
-- CLEANUP
set client_min_messages='warning';
drop schema bfv_aggregate cascade;
69 changes: 68 additions & 1 deletion src/test/regress/expected/bfv_aggregate_optimizer.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2139,9 +2139,11 @@ explain (costs off) select * from t4 group by a, b, c;
QUERY PLAN
------------------------------------------
Gather Motion 3:1 (slice1; segments: 3)
-> GroupAggregate
Group Key: a, b, c
-> Seq Scan on t4
Optimizer: Pivotal Optimizer (GPORCA)
(3 rows)
(5 rows)

explain (costs off) select * from t5 group by a, b, c;
QUERY PLAN
Expand Down Expand Up @@ -2193,6 +2195,71 @@ commit;
ERROR: duplicate key value violates unique constraint "t3_pkey" (seg1 127.0.0.1:7003 pid=89310)
DETAIL: Key (a, b)=(1, 1) already exists.
drop table t1, t2, t3, t4, t5, t6;
-- Test NULLs in a UNIQUE key column. A UNIQUE constraint allows multiple NULLs
-- (NULL is never "equal" for uniqueness checks), but grouping columns that
-- cover such a key must still collapse those NULLs into a single row, so
-- dropping the GbAgg is only safe when every key column is also NOT NULL.
drop table if exists t7, t8, t9;
NOTICE: table "t7" does not exist, skipping
NOTICE: table "t8" does not exist, skipping
NOTICE: table "t9" does not exist, skipping
create table t7 (a int unique, b int);
create table t8 (a int unique not null, b int);
create table t9 (a int primary key, b int);
insert into t7 values (1, 1), (2, 2), (null, 3), (null, 4), (null, 5);
insert into t8 values (1, 1), (2, 2), (3, 3);
insert into t9 values (1, 1), (2, 2), (3, 3);
explain (costs off) select distinct a from t7;
QUERY PLAN
------------------------------------------
Gather Motion 3:1 (slice1; segments: 3)
-> GroupAggregate
Group Key: a
-> Seq Scan on t7
Optimizer: GPORCA
(5 rows)

select distinct a from t7 order by a;
a
---
1
2

(3 rows)

explain (costs off) select distinct a from t8;
QUERY PLAN
------------------------------------------
Gather Motion 3:1 (slice1; segments: 3)
-> Seq Scan on t8
Optimizer: GPORCA
(3 rows)

select distinct a from t8 order by a;
a
---
1
2
3
(3 rows)

explain (costs off) select distinct a from t9;
QUERY PLAN
------------------------------------------
Gather Motion 3:1 (slice1; segments: 3)
-> Seq Scan on t9
Optimizer: GPORCA
(3 rows)

select distinct a from t9 order by a;
a
---
1
2
3
(3 rows)

drop table t7, t8, t9;
-- CLEANUP
set client_min_messages='warning';
drop schema bfv_aggregate cascade;
25 changes: 25 additions & 0 deletions src/test/regress/sql/bfv_aggregate.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1555,6 +1555,31 @@ commit;

drop table t1, t2, t3, t4, t5, t6;

-- Test NULLs in a UNIQUE key column. A UNIQUE constraint allows multiple NULLs
-- (NULL is never "equal" for uniqueness checks), but grouping columns that
-- cover such a key must still collapse those NULLs into a single row, so
-- dropping the GbAgg is only safe when every key column is also NOT NULL.

drop table if exists t7, t8, t9;
create table t7 (a int unique, b int);
create table t8 (a int unique not null, b int);
create table t9 (a int primary key, b int);

insert into t7 values (1, 1), (2, 2), (null, 3), (null, 4), (null, 5);
insert into t8 values (1, 1), (2, 2), (3, 3);
insert into t9 values (1, 1), (2, 2), (3, 3);

explain (costs off) select distinct a from t7;
select distinct a from t7 order by a;

explain (costs off) select distinct a from t8;
select distinct a from t8 order by a;

explain (costs off) select distinct a from t9;
select distinct a from t9 order by a;

drop table t7, t8, t9;

-- CLEANUP
set client_min_messages='warning';
drop schema bfv_aggregate cascade;
Loading