opt: simplify COALESCE using NOT NULL column derivation#171887
opt: simplify COALESCE using NOT NULL column derivation#171887lohitkolluri wants to merge 4 commits into
Conversation
|
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. |
|
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) |
|
Thank you for updating your pull request. Before a member of our team reviews your PR, I have some potential action items for you:
🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
Thank you for updating your pull request. Before a member of our team reviews your PR, I have some potential action items for you:
🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
Thank you for updating your pull request. Before a member of our team reviews your PR, I have some potential action items for you:
🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
8761f04 to
8b3ff53
Compare
|
Thank you for updating your pull request. Before a member of our team reviews your PR, I have some potential action items for you:
🦉 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>
8b3ff53 to
c4e5311
Compare
|
Thank you for updating your pull request. Before a member of our team reviews your PR, I have some potential action items for you:
🦉 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>
c4e5311 to
051e071
Compare
|
Thank you for updating your pull request. Before a member of our team reviews your PR, I have some potential action items for you:
🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
@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! |
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) = 1→b = 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.