Skip to content

Commit 4290f69

Browse files
Marios Trivyzasmatriv
Marios Trivyzas
authored andcommitted
SQL: Fix edge case: <field> IN (null) (#34802)
Handle the case when `null` is the only value in the list so that it's translated to a `MatchNoDocsQuery`. Followup to: #34750
1 parent 8d83b2d commit 4290f69

File tree

5 files changed

+41
-6
lines changed

5 files changed

+41
-6
lines changed

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/In.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,19 @@ public boolean nullable() {
7878

7979
@Override
8080
public boolean foldable() {
81-
return Expressions.foldable(children());
81+
return Expressions.foldable(children()) ||
82+
(Expressions.foldable(list) && list().stream().allMatch(e -> e.dataType() == DataType.NULL));
8283
}
8384

8485
@Override
8586
public Boolean fold() {
87+
if (value.dataType() == DataType.NULL) {
88+
return null;
89+
}
90+
if (list.size() == 1 && list.get(0).dataType() == DataType.NULL) {
91+
return false;
92+
}
93+
8694
Object foldedLeftValue = value.fold();
8795
Boolean result = false;
8896
for (Expression rightValue : list) {

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/TermsQuery.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,23 +11,28 @@
1111
import org.elasticsearch.xpack.sql.tree.Location;
1212
import org.elasticsearch.xpack.sql.type.DataType;
1313

14+
import java.util.Collections;
1415
import java.util.LinkedHashSet;
1516
import java.util.List;
1617
import java.util.Objects;
18+
import java.util.Set;
1719

1820
import static org.elasticsearch.index.query.QueryBuilders.termsQuery;
1921

2022
public class TermsQuery extends LeafQuery {
2123

2224
private final String term;
23-
private final LinkedHashSet<Object> values;
25+
private final Set<Object> values;
2426

2527
public TermsQuery(Location location, String term, List<Expression> values) {
2628
super(location);
2729
this.term = term;
2830
values.removeIf(e -> e.dataType() == DataType.NULL);
29-
this.values = new LinkedHashSet<>(Foldables.valuesOf(values, values.get(0).dataType()));
30-
this.values.removeIf(Objects::isNull);
31+
if (values.isEmpty()) {
32+
this.values = Collections.emptySet();
33+
} else {
34+
this.values = new LinkedHashSet<>(Foldables.valuesOf(values, values.get(0).dataType()));
35+
}
3136
}
3237

3338
@Override

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public class OptimizerTests extends ESTestCase {
9595
private static final Literal FOUR = L(4);
9696
private static final Literal FIVE = L(5);
9797
private static final Literal SIX = L(6);
98-
98+
private static final Literal NULL = L(null);
9999

100100
public static class DummyBooleanExpression extends Expression {
101101

@@ -323,6 +323,18 @@ public void testConstantFoldingIn_LeftValueNotFoldable() {
323323
assertThat(Foldables.valuesOf(in.list(), DataType.INTEGER), contains(1 ,2 ,3 ,4));
324324
}
325325

326+
public void testConstantFoldingIn_RightValueIsNull() {
327+
In in = new In(EMPTY, getFieldAttribute(), Arrays.asList(NULL, NULL));
328+
Literal result= (Literal) new ConstantFolding().rule(in);
329+
assertEquals(false, result.value());
330+
}
331+
332+
public void testConstantFoldingIn_LeftValueIsNull() {
333+
In in = new In(EMPTY, NULL, Arrays.asList(ONE, TWO, THREE));
334+
Literal result= (Literal) new ConstantFolding().rule(in);
335+
assertNull(result.value());
336+
}
337+
326338
public void testArithmeticFolding() {
327339
assertEquals(10, foldOperator(new Add(EMPTY, L(7), THREE)));
328340
assertEquals(4, foldOperator(new Sub(EMPTY, L(7), THREE)));

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,16 @@ public void testFoldingToLocalExecWithProject() {
6464
assertThat(ee.output().get(0).toString(), startsWith("keyword{f}#"));
6565
}
6666

67+
public void testFoldingToLocalExecWithProject_FoldableIn() {
68+
PhysicalPlan p = plan("SELECT keyword FROM test WHERE int IN (null, null)");
69+
assertEquals(LocalExec.class, p.getClass());
70+
LocalExec le = (LocalExec) p;
71+
assertEquals(EmptyExecutable.class, le.executable().getClass());
72+
EmptyExecutable ee = (EmptyExecutable) le.executable();
73+
assertEquals(1, ee.output().size());
74+
assertThat(ee.output().get(0).toString(), startsWith("keyword{f}#"));
75+
}
76+
6777
public void testFoldingToLocalExecWithProject_WithOrderAndLimit() {
6878
PhysicalPlan p = plan("SELECT keyword FROM test WHERE 1 = 2 ORDER BY int LIMIT 10");
6979
assertEquals(LocalExec.class, p.getClass());

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ public void testTranslateInExpression_WhereClause() throws IOException {
173173
assertEquals("keyword:(bar foo lala)", tq.asBuilder().toQuery(createShardContext()).toString());
174174
}
175175

176-
public void testTranslateInExpression_WhereClauseAndNullHAndling() throws IOException {
176+
public void testTranslateInExpression_WhereClauseAndNullHandling() throws IOException {
177177
LogicalPlan p = plan("SELECT * FROM test WHERE keyword IN ('foo', null, 'lala', null, 'foo', concat('la', 'la'))");
178178
assertTrue(p instanceof Project);
179179
assertTrue(p.children().get(0) instanceof Filter);

0 commit comments

Comments
 (0)