Skip to content

Fix ORCA GbAgg drop for nullable UNIQUE keys - #1941

Open
roseduan wants to merge 1 commit into
apache:mainfrom
roseduan:fix_orca_nullable_unqiue_index_agg
Open

Fix ORCA GbAgg drop for nullable UNIQUE keys#1941
roseduan wants to merge 1 commit into
apache:mainfrom
roseduan:fix_orca_nullable_unqiue_index_agg

Conversation

@roseduan

Copy link
Copy Markdown
Contributor

CXformSimplifyGbAgg::FDropGbAgg() drops a GROUP BY/DISTINCT's aggregate node when the grouping columns cover a key derived from a UNIQUE or PRIMARY KEY constraint, assuming the key already guarantees one row per value.

That's wrong for a plain UNIQUE column: SQL allows multiple NULLs in it (NULL is never "equal" for uniqueness checks), but dedup must still collapse those NULLs into one row.

Before:
create table repro (c0 numeric unique);
insert into repro values (1), (2), (null), (null), (null);
explain (costs off) select distinct c0 from repro;
Gather Motion 3:1 (slice1; segments: 3)
-> Seq Scan on repro -- no aggregate, all 5 rows returned

After:
Gather Motion 3:1 (slice1; segments: 3)
-> GroupAggregate
Group Key: repro.c0
-> Seq Scan on repro -- correctly returns 1, 2, NULL

Fix by also requiring every key column to be provably NOT NULL (CExpression::DeriveNotNullColumns()) before dropping the aggregate. PRIMARY KEY columns are always NOT NULL, so that case is unaffected.

Fixes #ISSUE_Number

What does this PR do?

Type of Change

  • Bug fix (non-breaking change)
  • New feature (non-breaking change)
  • Breaking change (fix or feature with breaking changes)
  • Documentation update

Breaking Changes

Test Plan

  • Unit tests added/updated
  • Integration tests added/updated
  • Passed make installcheck
  • Passed make -C src/test installcheck-cbdb-parallel

Impact

Performance:

User-facing changes:

Dependencies:

Checklist

Additional Context

CI Skip Instructions


CXformSimplifyGbAgg::FDropGbAgg() drops a GROUP BY/DISTINCT's
aggregate node when the grouping columns cover a key derived from
a UNIQUE or PRIMARY KEY constraint, assuming the key already
guarantees one row per value.

That's wrong for a plain UNIQUE column: SQL allows multiple NULLs
in it (NULL is never "equal" for uniqueness checks), but dedup
must still collapse those NULLs into one row.

Before:
    create table repro (c0 numeric unique);
    insert into repro values (1), (2), (null), (null), (null);
    explain (costs off) select distinct c0 from repro;
      Gather Motion 3:1  (slice1; segments: 3)
        ->  Seq Scan on repro          -- no aggregate, all 5 rows returned

After:
      Gather Motion 3:1  (slice1; segments: 3)
        ->  GroupAggregate
              Group Key: repro.c0
              ->  Seq Scan on repro    -- correctly returns 1, 2, NULL

Fix by also requiring every key column to be provably NOT NULL
(CExpression::DeriveNotNullColumns()) before dropping the
aggregate. PRIMARY KEY columns are always NOT NULL, so that case
is unaffected.
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;

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 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants