Skip to content

Commit 038d0b4

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

File tree

7 files changed

+747
-17
lines changed

7 files changed

+747
-17
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
@@ -814,3 +814,277 @@ COUNT_DISTINCT(c):long|COUNT_DISTINCT(null):long|COUNT_DISTINCT(null - 1):long|a
814814
0 |0 |0 |2
815815
0 |0 |0 |3
816816
;
817+
818+
// Tests for fix: (a IS NOT NULL OR p) AND a IS NULL must not discard the surviving OR branch p.
819+
// Without the fix, PropagateNullable.nullify() replaced the entire OR with null, returning 0 rows.
820+
821+
isNullWithOrDisjunctionAllRows
822+
required_capability: fix_propagate_nullable_or_disjunction
823+
824+
FROM employees
825+
| WHERE (gender IS NOT NULL OR emp_no > 10000) AND gender IS NULL
826+
| STATS c = COUNT(*)
827+
;
828+
829+
c:long
830+
10
831+
;
832+
833+
isNullWithOrDisjunctionSurvivingBranchFilters
834+
required_capability: fix_propagate_nullable_or_disjunction
835+
836+
FROM employees
837+
| WHERE (gender IS NOT NULL OR emp_no > 10015) AND gender IS NULL
838+
| SORT emp_no
839+
| KEEP emp_no, gender
840+
;
841+
842+
emp_no:integer|gender:keyword
843+
10016 |null
844+
10017 |null
845+
10018 |null
846+
10019 |null
847+
;
848+
849+
isNullWithNestedOrDisjunction
850+
required_capability: fix_propagate_nullable_or_disjunction
851+
852+
FROM employees
853+
| WHERE emp_no > 10000 AND (gender IS NOT NULL OR languages > 4) AND gender IS NULL
854+
| SORT emp_no
855+
| KEEP emp_no, gender
856+
;
857+
858+
emp_no:integer|gender:keyword
859+
10011 |null
860+
10012 |null
861+
10014 |null
862+
10015 |null
863+
;
864+
865+
isNullWithIsNullInsideOr
866+
required_capability: fix_propagate_nullable_or_disjunction
867+
868+
FROM employees
869+
| WHERE (gender IS NULL OR emp_no > 10000) AND gender IS NULL
870+
| STATS c = COUNT(*)
871+
;
872+
873+
c:long
874+
10
875+
;
876+
877+
isNullWithOrSalaryFilter
878+
required_capability: fix_propagate_nullable_or_disjunction
879+
880+
FROM employees
881+
| WHERE (gender IS NOT NULL OR salary > 50000) AND gender IS NULL
882+
| SORT emp_no
883+
| KEEP emp_no, gender, salary
884+
;
885+
886+
emp_no:integer|gender:keyword|salary:integer
887+
10016 |null |61358
888+
10017 |null |58715
889+
10018 |null |56760
890+
10019 |null |73717
891+
;
892+
893+
isNullWithOrLanguageFilter
894+
required_capability: fix_propagate_nullable_or_disjunction
895+
896+
FROM employees
897+
| WHERE (gender IS NOT NULL OR languages > 3) AND gender IS NULL
898+
| SORT emp_no
899+
| KEEP emp_no, gender, languages
900+
;
901+
902+
emp_no:integer|gender:keyword|languages:integer
903+
10010 |null |4
904+
10011 |null |5
905+
10012 |null |5
906+
10014 |null |5
907+
10015 |null |5
908+
;
909+
910+
isNullWithOrDisjunctionSeparateWhereClauses
911+
required_capability: fix_propagate_nullable_or_disjunction
912+
913+
FROM employees
914+
| WHERE gender IS NOT NULL OR emp_no > 10015
915+
| WHERE gender IS NULL
916+
| SORT emp_no
917+
| KEEP emp_no, gender
918+
;
919+
920+
emp_no:integer|gender:keyword
921+
10016 |null
922+
10017 |null
923+
10018 |null
924+
10019 |null
925+
;
926+
927+
isNullWithOrDisjunctionStatsByLanguage
928+
required_capability: fix_propagate_nullable_or_disjunction
929+
required_capability: inline_stats
930+
931+
FROM employees
932+
| WHERE (gender IS NOT NULL OR languages > 3) AND gender IS NULL
933+
| INLINE STATS c = COUNT(*) BY languages
934+
| SORT emp_no
935+
| KEEP emp_no, gender, languages, c
936+
;
937+
938+
emp_no:integer|gender:keyword|languages:integer|c:long
939+
10010 |null |4 |1
940+
10011 |null |5 |4
941+
10012 |null |5 |4
942+
10014 |null |5 |4
943+
10015 |null |5 |4
944+
;
945+
946+
isNullWithOrThreeBranches
947+
required_capability: fix_propagate_nullable_or_disjunction
948+
949+
FROM employees
950+
| WHERE (gender IS NOT NULL OR emp_no > 10017 OR salary > 60000) AND gender IS NULL
951+
| SORT emp_no
952+
| KEEP emp_no, gender, salary
953+
;
954+
955+
emp_no:integer|gender:keyword|salary:integer
956+
10016 |null |61358
957+
10018 |null |56760
958+
10019 |null |73717
959+
;
960+
961+
isNullWithOrEvalAlias
962+
required_capability: fix_propagate_nullable_or_disjunction
963+
964+
FROM employees
965+
| EVAL g = gender
966+
| WHERE (g IS NOT NULL OR emp_no > 10015) AND g IS NULL
967+
| SORT emp_no
968+
| KEEP emp_no, g
969+
;
970+
971+
emp_no:integer|g:keyword
972+
10016 |null
973+
10017 |null
974+
10018 |null
975+
10019 |null
976+
;
977+
978+
isNullWithOrCompoundAndRangeEvalSeparateWhereClauses
979+
required_capability: fix_propagate_nullable_or_disjunction
980+
981+
FROM employees
982+
| EVAL nullable = gender
983+
| KEEP emp_no, nullable, gender
984+
| WHERE nullable IS NOT NULL OR (emp_no > 10010 AND emp_no <= 10016)
985+
| WHERE nullable IS NULL
986+
| SORT emp_no
987+
;
988+
989+
emp_no:integer|nullable:keyword|gender:keyword
990+
10011 |null |null
991+
10012 |null |null
992+
10013 |null |null
993+
10014 |null |null
994+
10015 |null |null
995+
10016 |null |null
996+
;
997+
998+
isNullWithOrDisjunctionMvEvalNull
999+
required_capability: fix_propagate_nullable_or_disjunction
1000+
1001+
FROM employees
1002+
| WHERE emp_no >= 10009 AND emp_no <= 10015
1003+
| EVAL sc = salary_change + 1
1004+
| WHERE (sc IS NOT NULL OR emp_no > 10011) AND sc IS NULL
1005+
| SORT emp_no
1006+
| KEEP emp_no, salary_change, sc
1007+
;
1008+
warningRegex:Line \d+:\d+: evaluation of \[salary_change \+ 1\] failed, treating result as null\. Only first 20 failures recorded\.
1009+
warningRegex:Line \d+:\d+: java\.lang\.IllegalArgumentException: single-value function encountered multi-value
1010+
1011+
emp_no:integer|salary_change:double|sc:double
1012+
10013 |null |null
1013+
10014 |[-1.89, 9.07] |null
1014+
10015 |[12.4, 14.25] |null
1015+
;
1016+
1017+
isNullWithOrDisjunctionMvEvalNullSeparateWhereClauses
1018+
required_capability: fix_propagate_nullable_or_disjunction
1019+
1020+
FROM employees
1021+
| WHERE emp_no >= 10009 AND emp_no <= 10015
1022+
| EVAL sc = salary_change + 1
1023+
| WHERE sc IS NOT NULL OR emp_no > 10011
1024+
| WHERE sc IS NULL
1025+
| SORT emp_no
1026+
| KEEP emp_no, salary_change, sc
1027+
;
1028+
warningRegex:Line \d+:\d+: evaluation of \[salary_change \+ 1\] failed, treating result as null\. Only first 20 failures recorded\.
1029+
warningRegex:Line \d+:\d+: java\.lang\.IllegalArgumentException: single-value function encountered multi-value
1030+
1031+
emp_no:integer|salary_change:double|sc:double
1032+
10013 |null |null
1033+
10014 |[-1.89, 9.07] |null
1034+
10015 |[12.4, 14.25] |null
1035+
;
1036+
1037+
isNotNullWithOrDisjunctionSurvivingBranch
1038+
required_capability: fix_propagate_nullable_or_disjunction
1039+
1040+
FROM employees
1041+
| WHERE (gender IS NULL OR emp_no <= 10009) AND gender IS NOT NULL
1042+
| SORT emp_no
1043+
| KEEP emp_no, gender
1044+
;
1045+
1046+
emp_no:integer|gender:keyword
1047+
10001 |M
1048+
10002 |F
1049+
10003 |M
1050+
10004 |M
1051+
10005 |M
1052+
10006 |F
1053+
10007 |F
1054+
10008 |M
1055+
10009 |F
1056+
;
1057+
1058+
isNotNullWithIsNotNullInsideOr
1059+
required_capability: fix_propagate_nullable_or_disjunction
1060+
1061+
FROM employees
1062+
| WHERE (gender IS NOT NULL OR emp_no <= 10009) AND gender IS NOT NULL
1063+
| STATS c = COUNT(*)
1064+
;
1065+
1066+
c:long
1067+
90
1068+
;
1069+
1070+
isNotNullWithOrDisjunctionSeparateWhereClauses
1071+
required_capability: fix_propagate_nullable_or_disjunction
1072+
1073+
FROM employees
1074+
| WHERE gender IS NULL OR emp_no <= 10009
1075+
| WHERE gender IS NOT NULL
1076+
| SORT emp_no
1077+
| KEEP emp_no, gender
1078+
;
1079+
1080+
emp_no:integer|gender:keyword
1081+
10001 |M
1082+
10002 |F
1083+
10003 |M
1084+
10004 |M
1085+
10005 |M
1086+
10006 |F
1087+
10007 |F
1088+
10008 |M
1089+
10009 |F
1090+
;

‎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
@@ -1083,3 +1083,52 @@ ROW a = 12::long
10831083
_fork:keyword | x:long
10841084
fork1 | 12
10851085
;
1086+
1087+
propagateNullableOrDisjunctionUnmappedField
1088+
required_capability: optional_fields_nullify_tech_preview
1089+
required_capability: fix_propagate_nullable_or_disjunction
1090+
1091+
SET unmapped_fields="nullify"\;
1092+
FROM employees
1093+
| WHERE (foo IS NOT NULL OR emp_no > 10015) AND foo IS NULL
1094+
| STATS c = COUNT(*)
1095+
;
1096+
1097+
c:long
1098+
85
1099+
;
1100+
1101+
propagateNullableOrDisjunctionUnmappedFieldSeparateWhereClauses
1102+
required_capability: optional_fields_nullify_tech_preview
1103+
required_capability: fix_propagate_nullable_or_disjunction
1104+
1105+
SET unmapped_fields="nullify"\;
1106+
FROM employees
1107+
| WHERE foo IS NOT NULL OR emp_no > 10015
1108+
| WHERE foo IS NULL
1109+
| STATS c = COUNT(*)
1110+
;
1111+
1112+
c:long
1113+
85
1114+
;
1115+
1116+
propagateNullableOrDisjunctionUnmappedFieldEvalAlias
1117+
required_capability: optional_fields_nullify_tech_preview
1118+
required_capability: fix_propagate_nullable_or_disjunction
1119+
1120+
SET unmapped_fields="nullify"\;
1121+
FROM employees
1122+
| EVAL x = foo
1123+
| WHERE (x IS NOT NULL OR emp_no > 10095) AND x IS NULL
1124+
| SORT emp_no
1125+
| KEEP emp_no, foo, x
1126+
;
1127+
1128+
emp_no:integer | foo:null | x:null
1129+
10096 | null | null
1130+
10097 | null | null
1131+
10098 | null | null
1132+
10099 | null | null
1133+
10100 | null | null
1134+
;

‎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
@@ -2470,6 +2470,17 @@ public enum Cap {
24702470
*/
24712471
FIX_FULL_TEXT_FUNCTIONS_ON_CONSTANT_KEYWORD,
24722472

2473+
/**
2474+
* Fix for {@code PropagateNullable} incorrectly discarding surviving OR branches when
2475+
* a field is constrained by {@code IS NULL} or {@code IS NOT NULL} in the same AND conjunction.
2476+
* Previously {@code (a IS NOT NULL OR p) AND a IS NULL} was optimized to {@code null AND a IS NULL}
2477+
* (dropping {@code p}); now it correctly becomes {@code p AND a IS NULL}.
2478+
* Symmetric fix: {@code (a IS NULL OR p) AND a IS NOT NULL} now correctly becomes
2479+
* {@code p AND a IS NOT NULL} instead of remaining unoptimized.
2480+
* See https://github.com/elastic/elasticsearch/issues/141579
2481+
*/
2482+
FIX_PROPAGATE_NULLABLE_OR_DISJUNCTION,
2483+
24732484
// Last capability should still have a comma for fewer merge conflicts when adding new ones :)
24742485
// This comment prevents the semicolon from being on the previous capability when Spotless formats the file.
24752486
;

0 commit comments

Comments
 (0)