Fix nullable propagation in OR/IS NULL filters#142728
Draft
MattAlp wants to merge 6 commits intoelastic:mainfrom
Draft
Fix nullable propagation in OR/IS NULL filters#142728MattAlp wants to merge 6 commits intoelastic:mainfrom
MattAlp wants to merge 6 commits intoelastic:mainfrom
Conversation
Preserve IS NULL filter semantics when OR branches reference the same field by substituting null into expressions and folding, instead of replacing entire branches with null. Co-authored-by: Cursor <cursoragent@cursor.com>
Use a reduced disjunction/nullability case and assert plan semantics rather than matching the original fuzz-generated query shape. Co-authored-by: Cursor <cursoragent@cursor.com>
Align the regression with the optimizer test suite pattern by using an explicit typed plan chain while still asserting both preserved conjuncts after optimization. Co-authored-by: Cursor <cursoragent@cursor.com>
...sql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java
Show resolved
Hide resolved
...sql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java
Show resolved
Hide resolved
...c/test/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PropagateNullableTests.java
Show resolved
Hide resolved
...ql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PropagateNullable.java
Show resolved
Hide resolved
...ql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PropagateNullable.java
Show resolved
Hide resolved
...ql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PropagateNullable.java
Show resolved
Hide resolved
julian-elastic
requested changes
Feb 19, 2026
Contributor
julian-elastic
left a comment
There was a problem hiding this comment.
Overall the change looks good. I left some small suggestions to address before we check in.
Contributor
|
Also please add better comments in the PR with explanation. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PropagateNullablesofield IS NULLpropagation substitutesnullinto matching expressions and folds, instead of nullifying entire expression branches(field IS NOT NULL OR p) AND field IS NULL, which previously could be rewritten to an always-empty filterPropagateNullabletests to prevent reintroducing the pruning