From d931ed9924cec39e379eb668cc49fd977f4bc0d4 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Wed, 24 Oct 2018 15:14:48 +0200 Subject: [PATCH] SQL: Fix edge case: ` IN(null) Handle the case when `null` is the only value in the list so that it's translated to a `MatchNoDocsQuery`. Followup to: #34750 --- .../xpack/sql/querydsl/query/TermsQuery.java | 11 ++++++++--- .../xpack/sql/planner/QueryTranslatorTests.java | 16 +++++++++++++++- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/TermsQuery.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/TermsQuery.java index 4366e2d404c13a..91ea49a8a3ce3d 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/TermsQuery.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/TermsQuery.java @@ -11,23 +11,28 @@ import org.elasticsearch.xpack.sql.tree.Location; import org.elasticsearch.xpack.sql.type.DataType; +import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; import java.util.Objects; +import java.util.Set; import static org.elasticsearch.index.query.QueryBuilders.termsQuery; public class TermsQuery extends LeafQuery { private final String term; - private final LinkedHashSet values; + private final Set values; public TermsQuery(Location location, String term, List values) { super(location); this.term = term; values.removeIf(e -> e.dataType() == DataType.NULL); - this.values = new LinkedHashSet<>(Foldables.valuesOf(values, values.get(0).dataType())); - this.values.removeIf(Objects::isNull); + if (values.isEmpty()) { + this.values = Collections.emptySet(); + } else { + this.values = new LinkedHashSet<>(Foldables.valuesOf(values, values.get(0).dataType())); + } } @Override diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java index 95b9be33a12a05..54f6ed6480414f 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java @@ -173,7 +173,7 @@ public void testTranslateInExpression_WhereClause() throws IOException { assertEquals("keyword:(bar foo lala)", tq.asBuilder().toQuery(createShardContext()).toString()); } - public void testTranslateInExpression_WhereClauseAndNullHAndling() throws IOException { + public void testTranslateInExpression_WhereClauseAndNullHandling() throws IOException { LogicalPlan p = plan("SELECT * FROM test WHERE keyword IN ('foo', null, 'lala', null, 'foo', concat('la', 'la'))"); assertTrue(p instanceof Project); assertTrue(p.children().get(0) instanceof Filter); @@ -186,6 +186,20 @@ public void testTranslateInExpression_WhereClauseAndNullHAndling() throws IOExce assertEquals("keyword:(foo lala)", tq.asBuilder().toQuery(createShardContext()).toString()); } + public void testTranslateInExpression_WhereClauseAndNullHandlingEmptyValuesList() throws IOException { + LogicalPlan p = plan("SELECT * FROM test WHERE keyword IN (null)"); + assertTrue(p instanceof Project); + assertTrue(p.children().get(0) instanceof Filter); + Expression condition = ((Filter) p.children().get(0)).condition(); + assertFalse(condition.foldable()); + QueryTranslation translation = QueryTranslator.toQuery(condition, false); + Query query = translation.query; + assertTrue(query instanceof TermsQuery); + TermsQuery tq = (TermsQuery) query; + assertEquals("MatchNoDocsQuery(\"No terms supplied for \"terms\" query.\")", + tq.asBuilder().toQuery(createShardContext()).toString()); + } + public void testTranslateInExpressionInvalidValues_WhereClause() { LogicalPlan p = plan("SELECT * FROM test WHERE keyword IN ('foo', 'bar', keyword)"); assertTrue(p instanceof Project);