Skip to content

opt: simplify COALESCE using NOT NULL column derivation#171887

Open
lohitkolluri wants to merge 4 commits into
cockroachdb:masterfrom
lohitkolluri:opt-coalesce-not-null
Open

opt: simplify COALESCE using NOT NULL column derivation#171887
lohitkolluri wants to merge 4 commits into
cockroachdb:masterfrom
lohitkolluri:opt-coalesce-not-null

Conversation

@lohitkolluri

@lohitkolluri lohitkolluri commented Jun 22, 2026

Copy link
Copy Markdown

Fixes #103596

Adds normalization rules that simplify COALESCE expressions by dropping provably-non-null operands, using NOT NULL column information derived from relational properties and filter constraints. For example:

COALESCE(NULL, x, y) → COALESCE(x, y)
COALESCE(x::INT, y) → x::INT (if x is provably non-null)

The simplification works at two levels: (1) a recursive walk over scalar expressions that catches nested COALESCE and sub-expressions, and (2) filter-level analysis that derives NOT NULL columns from other filter items' constraints (e.g., b > 0 AND COALESCE(b, c) = 1b = 1).

Epic: none
Release note (sql change): The optimizer can now simplify COALESCE expressions by dropping operands that are provably non-null, including from filter conditions using cross-filter NOT NULL derivation.

@lohitkolluri lohitkolluri requested a review from a team as a code owner June 22, 2026 22:48
@lohitkolluri lohitkolluri requested review from michae2 and removed request for a team June 22, 2026 22:48
@blathers-crl

blathers-crl Bot commented Jun 22, 2026

Copy link
Copy Markdown

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@blathers-crl blathers-crl Bot added the O-community Originated from the community label Jun 22, 2026
@lohitkolluri

Copy link
Copy Markdown
Author

hey @DrewKimball @mw5h @michae2 sorry for the trouble with my previous PR (#170858) — it had to be closed due to a mistake I made (shallow clone + push caused file bloat). I've recreated it cleanly from master with only the 5 intended implementation files and all the review feedback y'all raised is incorporated (function renames, removed unnecessary helpers, inlined optgen patterns, pointer-equality reuse for unchanged items)

@blathers-crl

blathers-crl Bot commented Jun 22, 2026

Copy link
Copy Markdown

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

  • We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@blathers-crl

blathers-crl Bot commented Jun 22, 2026

Copy link
Copy Markdown

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

  • We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@lohitkolluri lohitkolluri changed the title opt: drop provably-non-null COALESCE operands opt: simplify COALESCE using NOT NULL column derivation Jun 22, 2026
@blathers-crl

blathers-crl Bot commented Jun 22, 2026

Copy link
Copy Markdown

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

  • We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@lohitkolluri lohitkolluri force-pushed the opt-coalesce-not-null branch from 8761f04 to 8b3ff53 Compare June 22, 2026 23:57
@blathers-crl

blathers-crl Bot commented Jun 22, 2026

Copy link
Copy Markdown

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

  • We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

This commit adds normalization rules to simplify COALESCE expressions when
one or more operands are provably non-null. For example:

  COALESCE(NULL, x, y)  ->  COALESCE(x, y)
  COALESCE(x::INT, y)   ->  x::INT (if x is non-null)

The key changes are:

- Added CanSimplifyCoalesceInScalar helper function that checks whether
  a COALESCE expression has any provably-non-null operands
- Added SimplifyCoalesceInProjections and SimplifyCoalesceInFilters
  normalization rules
- Updated optgen patterns for project.opt and select.opt to use
  inlined match patterns

This is a minor optimization that can eliminate unnecessary COALESCE
evaluations during query planning.

Epic: none
Release note (sql change): The optimizer can now simplify COALESCE
expressions by dropping operands that are provably non-null.

Signed-off-by: Lohit Kolluri <lohitkolluri@gmail.com>
@lohitkolluri lohitkolluri force-pushed the opt-coalesce-not-null branch from 8b3ff53 to c4e5311 Compare June 22, 2026 23:59
@blathers-crl

blathers-crl Bot commented Jun 22, 2026

Copy link
Copy Markdown

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

  • We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

…lification

Updates test expectations for the new SimplifyCoalesceProject and
SimplifyCoalesceSelect normalization rules:

- pkg/sql/opt/norm/testdata/rules/scalar: new test cases for
  SimplifyCoalesceProject (positive and negative cases, nested COALESCE)
- pkg/sql/opt/norm/testdata/rules/select: new test cases for
  SimplifyCoalesceSelect (positive and negative cases, nested COALESCE)
- pkg/sql/opt/norm/testdata/rules/decorrelate: disable SimplifyCoalesceProject
  for an existing decorrelation test to preserve expected output
- pkg/sql/opt/xform/testdata/external/tpcds/q41-q50 and
  pkg/sql/opt/xform/testdata/external/tpcds/q41-q50-no-stats: columns that
  were previously COALESCE-wrapped are now provably non-null (!null annotation)

Epic: none
Signed-off-by: Lohit Kolluri <lohitkolluri@gmail.com>
…ct filters

Previously, SimplifyCoalesceSelect only considered NOT NULL columns
inferred from the input expression (e.g., Join conditions). This
meant COALESCE(b, c) could not be simplified to b in a query like:

  SELECT * FROM d WHERE b > 0 AND COALESCE(b, c) = 1

even though b > 0 null-rejects column b.

Now compute NOT NULL columns from all other filter items' constraints
and feed them into COALESCE simplification. Also refactors
CanSimplifyCoalesceInScalar from Replace-based recursion to a direct
recursive walk with proper ScalarExpr type assertion (fixes a panic
when encountering subquery relational children).

Epic: none
Signed-off-by: Lohit Kolluri <lohitkolluri@gmail.com>
…rivation

Extract the duplicate filter-derived NOT NULL computation into
computeFilterNotNullCols, used by both CanSimplifyCoalesceInFilters and
SimplifyCoalesceInFilters. Optimize the enriched NOT NULL set computation
to avoid ColSet::Union when the cross-filter difference is empty. Update
rule and test comments for accuracy.

Epic: none
Signed-off-by: Lohit Kolluri <lohitkolluri@gmail.com>
@lohitkolluri lohitkolluri force-pushed the opt-coalesce-not-null branch from c4e5311 to 051e071 Compare June 23, 2026 00:08
@blathers-crl

blathers-crl Bot commented Jun 23, 2026

Copy link
Copy Markdown

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

  • We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@lohitkolluri

Copy link
Copy Markdown
Author

@michae2 Hi! Just following up on this PR to see if you've had a chance to review it. No rush, just wanted to check in. Thanks!

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

Labels

O-community Originated from the community

Projects

None yet

Development

Successfully merging this pull request may close these issues.

opt: eliminate all arguments of coalesce after the first argument that is guaranteed to be non-null

1 participant