Skip to content

Commit 5e0b2a4

Browse files
astefanastefan
committed
ESQL: Do not discard disjunction conditions when is null/is not null might invalidate them (elastic#145941)
This is closing (not fixing, because there is no reproduceable use case, nor there is a possible java execution path for that) elastic#141579. AI-assisted PR. (cherry picked from commit 038d0b4)
1 parent 30ae22b commit 5e0b2a4

File tree

7 files changed

+747
-19
lines changed

7 files changed

+747
-19
lines changed

‎docs/changelog/145941.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
area: ES|QL
2+
issues: []
3+
pr: 145941
4+
summary: Do not discard disjunction conditions when is null/is not null might invalidate
5+
them
6+
type: bug

‎x-pack/plugin/esql/qa/testFixtures/src/main/resources/null.csv-spec‎

Lines changed: 274 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -584,3 +584,277 @@ FROM employees | WHERE emp_no IN (10001, null) AND first_name IS NOT NULL | SORT
584584
emp_no:integer | first_name:keyword
585585
10001 | Georgi
586586
;
587+
588+
// Tests for fix: (a IS NOT NULL OR p) AND a IS NULL must not discard the surviving OR branch p.
589+
// Without the fix, PropagateNullable.nullify() replaced the entire OR with null, returning 0 rows.
590+
591+
isNullWithOrDisjunctionAllRows
592+
required_capability: fix_propagate_nullable_or_disjunction
593+
594+
FROM employees
595+
| WHERE (gender IS NOT NULL OR emp_no > 10000) AND gender IS NULL
596+
| STATS c = COUNT(*)
597+
;
598+
599+
c:long
600+
10
601+
;
602+
603+
isNullWithOrDisjunctionSurvivingBranchFilters
604+
required_capability: fix_propagate_nullable_or_disjunction
605+
606+
FROM employees
607+
| WHERE (gender IS NOT NULL OR emp_no > 10015) AND gender IS NULL
608+
| SORT emp_no
609+
| KEEP emp_no, gender
610+
;
611+
612+
emp_no:integer|gender:keyword
613+
10016 |null
614+
10017 |null
615+
10018 |null
616+
10019 |null
617+
;
618+
619+
isNullWithNestedOrDisjunction
620+
required_capability: fix_propagate_nullable_or_disjunction
621+
622+
FROM employees
623+
| WHERE emp_no > 10000 AND (gender IS NOT NULL OR languages > 4) AND gender IS NULL
624+
| SORT emp_no
625+
| KEEP emp_no, gender
626+
;
627+
628+
emp_no:integer|gender:keyword
629+
10011 |null
630+
10012 |null
631+
10014 |null
632+
10015 |null
633+
;
634+
635+
isNullWithIsNullInsideOr
636+
required_capability: fix_propagate_nullable_or_disjunction
637+
638+
FROM employees
639+
| WHERE (gender IS NULL OR emp_no > 10000) AND gender IS NULL
640+
| STATS c = COUNT(*)
641+
;
642+
643+
c:long
644+
10
645+
;
646+
647+
isNullWithOrSalaryFilter
648+
required_capability: fix_propagate_nullable_or_disjunction
649+
650+
FROM employees
651+
| WHERE (gender IS NOT NULL OR salary > 50000) AND gender IS NULL
652+
| SORT emp_no
653+
| KEEP emp_no, gender, salary
654+
;
655+
656+
emp_no:integer|gender:keyword|salary:integer
657+
10016 |null |61358
658+
10017 |null |58715
659+
10018 |null |56760
660+
10019 |null |73717
661+
;
662+
663+
isNullWithOrLanguageFilter
664+
required_capability: fix_propagate_nullable_or_disjunction
665+
666+
FROM employees
667+
| WHERE (gender IS NOT NULL OR languages > 3) AND gender IS NULL
668+
| SORT emp_no
669+
| KEEP emp_no, gender, languages
670+
;
671+
672+
emp_no:integer|gender:keyword|languages:integer
673+
10010 |null |4
674+
10011 |null |5
675+
10012 |null |5
676+
10014 |null |5
677+
10015 |null |5
678+
;
679+
680+
isNullWithOrDisjunctionSeparateWhereClauses
681+
required_capability: fix_propagate_nullable_or_disjunction
682+
683+
FROM employees
684+
| WHERE gender IS NOT NULL OR emp_no > 10015
685+
| WHERE gender IS NULL
686+
| SORT emp_no
687+
| KEEP emp_no, gender
688+
;
689+
690+
emp_no:integer|gender:keyword
691+
10016 |null
692+
10017 |null
693+
10018 |null
694+
10019 |null
695+
;
696+
697+
isNullWithOrDisjunctionStatsByLanguage
698+
required_capability: fix_propagate_nullable_or_disjunction
699+
required_capability: inline_stats
700+
701+
FROM employees
702+
| WHERE (gender IS NOT NULL OR languages > 3) AND gender IS NULL
703+
| INLINE STATS c = COUNT(*) BY languages
704+
| SORT emp_no
705+
| KEEP emp_no, gender, languages, c
706+
;
707+
708+
emp_no:integer|gender:keyword|languages:integer|c:long
709+
10010 |null |4 |1
710+
10011 |null |5 |4
711+
10012 |null |5 |4
712+
10014 |null |5 |4
713+
10015 |null |5 |4
714+
;
715+
716+
isNullWithOrThreeBranches
717+
required_capability: fix_propagate_nullable_or_disjunction
718+
719+
FROM employees
720+
| WHERE (gender IS NOT NULL OR emp_no > 10017 OR salary > 60000) AND gender IS NULL
721+
| SORT emp_no
722+
| KEEP emp_no, gender, salary
723+
;
724+
725+
emp_no:integer|gender:keyword|salary:integer
726+
10016 |null |61358
727+
10018 |null |56760
728+
10019 |null |73717
729+
;
730+
731+
isNullWithOrEvalAlias
732+
required_capability: fix_propagate_nullable_or_disjunction
733+
734+
FROM employees
735+
| EVAL g = gender
736+
| WHERE (g IS NOT NULL OR emp_no > 10015) AND g IS NULL
737+
| SORT emp_no
738+
| KEEP emp_no, g
739+
;
740+
741+
emp_no:integer|g:keyword
742+
10016 |null
743+
10017 |null
744+
10018 |null
745+
10019 |null
746+
;
747+
748+
isNullWithOrCompoundAndRangeEvalSeparateWhereClauses
749+
required_capability: fix_propagate_nullable_or_disjunction
750+
751+
FROM employees
752+
| EVAL nullable = gender
753+
| KEEP emp_no, nullable, gender
754+
| WHERE nullable IS NOT NULL OR (emp_no > 10010 AND emp_no <= 10016)
755+
| WHERE nullable IS NULL
756+
| SORT emp_no
757+
;
758+
759+
emp_no:integer|nullable:keyword|gender:keyword
760+
10011 |null |null
761+
10012 |null |null
762+
10013 |null |null
763+
10014 |null |null
764+
10015 |null |null
765+
10016 |null |null
766+
;
767+
768+
isNullWithOrDisjunctionMvEvalNull
769+
required_capability: fix_propagate_nullable_or_disjunction
770+
771+
FROM employees
772+
| WHERE emp_no >= 10009 AND emp_no <= 10015
773+
| EVAL sc = salary_change + 1
774+
| WHERE (sc IS NOT NULL OR emp_no > 10011) AND sc IS NULL
775+
| SORT emp_no
776+
| KEEP emp_no, salary_change, sc
777+
;
778+
warningRegex:Line \d+:\d+: evaluation of \[salary_change \+ 1\] failed, treating result as null\. Only first 20 failures recorded\.
779+
warningRegex:Line \d+:\d+: java\.lang\.IllegalArgumentException: single-value function encountered multi-value
780+
781+
emp_no:integer|salary_change:double|sc:double
782+
10013 |null |null
783+
10014 |[-1.89, 9.07] |null
784+
10015 |[12.4, 14.25] |null
785+
;
786+
787+
isNullWithOrDisjunctionMvEvalNullSeparateWhereClauses
788+
required_capability: fix_propagate_nullable_or_disjunction
789+
790+
FROM employees
791+
| WHERE emp_no >= 10009 AND emp_no <= 10015
792+
| EVAL sc = salary_change + 1
793+
| WHERE sc IS NOT NULL OR emp_no > 10011
794+
| WHERE sc IS NULL
795+
| SORT emp_no
796+
| KEEP emp_no, salary_change, sc
797+
;
798+
warningRegex:Line \d+:\d+: evaluation of \[salary_change \+ 1\] failed, treating result as null\. Only first 20 failures recorded\.
799+
warningRegex:Line \d+:\d+: java\.lang\.IllegalArgumentException: single-value function encountered multi-value
800+
801+
emp_no:integer|salary_change:double|sc:double
802+
10013 |null |null
803+
10014 |[-1.89, 9.07] |null
804+
10015 |[12.4, 14.25] |null
805+
;
806+
807+
isNotNullWithOrDisjunctionSurvivingBranch
808+
required_capability: fix_propagate_nullable_or_disjunction
809+
810+
FROM employees
811+
| WHERE (gender IS NULL OR emp_no <= 10009) AND gender IS NOT NULL
812+
| SORT emp_no
813+
| KEEP emp_no, gender
814+
;
815+
816+
emp_no:integer|gender:keyword
817+
10001 |M
818+
10002 |F
819+
10003 |M
820+
10004 |M
821+
10005 |M
822+
10006 |F
823+
10007 |F
824+
10008 |M
825+
10009 |F
826+
;
827+
828+
isNotNullWithIsNotNullInsideOr
829+
required_capability: fix_propagate_nullable_or_disjunction
830+
831+
FROM employees
832+
| WHERE (gender IS NOT NULL OR emp_no <= 10009) AND gender IS NOT NULL
833+
| STATS c = COUNT(*)
834+
;
835+
836+
c:long
837+
90
838+
;
839+
840+
isNotNullWithOrDisjunctionSeparateWhereClauses
841+
required_capability: fix_propagate_nullable_or_disjunction
842+
843+
FROM employees
844+
| WHERE gender IS NULL OR emp_no <= 10009
845+
| WHERE gender IS NOT NULL
846+
| SORT emp_no
847+
| KEEP emp_no, gender
848+
;
849+
850+
emp_no:integer|gender:keyword
851+
10001 |M
852+
10002 |F
853+
10003 |M
854+
10004 |M
855+
10005 |M
856+
10006 |F
857+
10007 |F
858+
10008 |M
859+
10009 |F
860+
;

‎x-pack/plugin/esql/qa/testFixtures/src/main/resources/unmapped-nullify.csv-spec‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -763,3 +763,52 @@ ROW a = 12::long
763763
_fork:keyword | x:long
764764
fork1 | 12
765765
;
766+
767+
propagateNullableOrDisjunctionUnmappedField
768+
required_capability: optional_fields_nullify_tech_preview
769+
required_capability: fix_propagate_nullable_or_disjunction
770+
771+
SET unmapped_fields="nullify"\;
772+
FROM employees
773+
| WHERE (foo IS NOT NULL OR emp_no > 10015) AND foo IS NULL
774+
| STATS c = COUNT(*)
775+
;
776+
777+
c:long
778+
85
779+
;
780+
781+
propagateNullableOrDisjunctionUnmappedFieldSeparateWhereClauses
782+
required_capability: optional_fields_nullify_tech_preview
783+
required_capability: fix_propagate_nullable_or_disjunction
784+
785+
SET unmapped_fields="nullify"\;
786+
FROM employees
787+
| WHERE foo IS NOT NULL OR emp_no > 10015
788+
| WHERE foo IS NULL
789+
| STATS c = COUNT(*)
790+
;
791+
792+
c:long
793+
85
794+
;
795+
796+
propagateNullableOrDisjunctionUnmappedFieldEvalAlias
797+
required_capability: optional_fields_nullify_tech_preview
798+
required_capability: fix_propagate_nullable_or_disjunction
799+
800+
SET unmapped_fields="nullify"\;
801+
FROM employees
802+
| EVAL x = foo
803+
| WHERE (x IS NOT NULL OR emp_no > 10095) AND x IS NULL
804+
| SORT emp_no
805+
| KEEP emp_no, foo, x
806+
;
807+
808+
emp_no:integer | foo:null | x:null
809+
10096 | null | null
810+
10097 | null | null
811+
10098 | null | null
812+
10099 | null | null
813+
10100 | null | null
814+
;

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1955,6 +1955,17 @@ public enum Cap {
19551955

19561956
KEYWORDS_MV_COUNT_AS_SINGLE_VALUE_FIX,
19571957

1958+
/**
1959+
* Fix for {@code PropagateNullable} incorrectly discarding surviving OR branches when
1960+
* a field is constrained by {@code IS NULL} or {@code IS NOT NULL} in the same AND conjunction.
1961+
* Previously {@code (a IS NOT NULL OR p) AND a IS NULL} was optimized to {@code null AND a IS NULL}
1962+
* (dropping {@code p}); now it correctly becomes {@code p AND a IS NULL}.
1963+
* Symmetric fix: {@code (a IS NULL OR p) AND a IS NOT NULL} now correctly becomes
1964+
* {@code p AND a IS NOT NULL} instead of remaining unoptimized.
1965+
* See https://github.com/elastic/elasticsearch/issues/141579
1966+
*/
1967+
FIX_PROPAGATE_NULLABLE_OR_DISJUNCTION,
1968+
19581969
// Last capability should still have a comma for fewer merge conflicts when adding new ones :)
19591970
// This comment prevents the semicolon from being on the previous capability when Spotless formats the file.
19601971
;

0 commit comments

Comments
 (0)