From 0eda16ca3a4166e2bdcc47882c55aaaa05588cfb Mon Sep 17 00:00:00 2001 From: kanoshiou Date: Fri, 28 Mar 2025 16:30:03 +0800 Subject: [PATCH 1/6] Fix expression null check --- .../xpack/esql/expression/predicate/operator/comparison/In.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/In.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/In.java index 6bd57a44c6131..5100da1dfa0b5 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/In.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/In.java @@ -478,7 +478,7 @@ private Query translate(TranslatorHandler handler) { List queries = new ArrayList<>(); for (Expression rhs : list()) { - if (DataType.isNull(rhs.dataType()) == false) { + if (Expressions.isGuaranteedNull(rhs) == false) { if (needsTypeSpecificValueHandling(attribute.dataType())) { // delegates to BinaryComparisons translator to ensure consistent handling of date and time values // TODO: From 11795aa00e8f5d2c7ef9a85c8b2a6595dc887b6d Mon Sep 17 00:00:00 2001 From: kanoshiou Date: Fri, 28 Mar 2025 17:00:35 +0800 Subject: [PATCH 2/6] Update docs/changelog/125832.yaml --- docs/changelog/125832.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 docs/changelog/125832.yaml diff --git a/docs/changelog/125832.yaml b/docs/changelog/125832.yaml new file mode 100644 index 0000000000000..4877a02e9e6d0 --- /dev/null +++ b/docs/changelog/125832.yaml @@ -0,0 +1,6 @@ +pr: 125832 +summary: "ESQL: Fix `NULL` handling in `IN` clause" +area: ES|QL +type: bug +issues: + - 119950 From 15622befa43949383c12a0d9c59d850a16d638c7 Mon Sep 17 00:00:00 2001 From: kanoshiou Date: Fri, 28 Mar 2025 17:31:11 +0800 Subject: [PATCH 3/6] Update csv test --- .../qa/testFixtures/src/main/resources/null.csv-spec | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/null.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/null.csv-spec index 7bf3bc7613e01..121d285b2b895 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/null.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/null.csv-spec @@ -203,3 +203,14 @@ FROM employees fullname:keyword | job_positions:keyword | salary:integer | salary_change:double ; + +inConvertedNull +FROM employees +| WHERE emp_no in (10021, 10022, null::int) +| KEEP emp_no, first_name, last_name +; + +emp_no:integer | first_name:keyword | last_name:keyword +10021 | Ramzi | Erde +10022 | Shahaf | Famili +; From 7b4499cdee7899af4c7f62c6744cc1dd801e39b4 Mon Sep 17 00:00:00 2001 From: kanoshiou Date: Fri, 28 Mar 2025 22:50:14 +0800 Subject: [PATCH 4/6] Update tests --- .../predicate/operator/comparison/InTests.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InTests.java index c286b2a1109cb..8def801de4325 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InTests.java @@ -13,12 +13,16 @@ import org.elasticsearch.geo.GeometryTestUtils; import org.elasticsearch.geo.ShapeTestUtils; import org.elasticsearch.xpack.esql.core.expression.Expression; +import org.elasticsearch.xpack.esql.core.expression.FieldAttribute; import org.elasticsearch.xpack.esql.core.expression.FoldContext; import org.elasticsearch.xpack.esql.core.expression.Literal; +import org.elasticsearch.xpack.esql.core.querydsl.query.TermsQuery; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; +import org.elasticsearch.xpack.esql.core.type.EsField; import org.elasticsearch.xpack.esql.expression.function.AbstractFunctionTestCase; import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier; +import org.elasticsearch.xpack.esql.planner.TranslatorHandler; import org.junit.AfterClass; import java.io.IOException; @@ -26,6 +30,8 @@ import java.util.Arrays; import java.util.List; import java.util.Locale; +import java.util.Map; +import java.util.Set; import java.util.function.Supplier; import java.util.stream.IntStream; @@ -81,6 +87,16 @@ private static Literal L(Object value) { return of(EMPTY, value); } + public void testConvertedNull() { + In in = new In( + EMPTY, + new FieldAttribute(Source.EMPTY, "field", new EsField("suffix", DataType.KEYWORD, Map.of(), true)), + Arrays.asList(ONE, new Literal(Source.EMPTY, null, DataType.INTEGER), THREE) + ); + var query = in.asQuery(TranslatorHandler.TRANSLATOR_HANDLER); + assertEquals(new TermsQuery(EMPTY, "field", Set.of(1, 3)), query); + } + @ParametersFactory public static Iterable parameters() { List suppliers = new ArrayList<>(); From 2fd4e60fd08b1c12d2e906c0ef6bd48d850607f1 Mon Sep 17 00:00:00 2001 From: kanoshiou Date: Wed, 9 Apr 2025 16:35:25 +0800 Subject: [PATCH 5/6] Fix bwc --- .../esql/qa/testFixtures/src/main/resources/null.csv-spec | 1 + .../elasticsearch/xpack/esql/action/EsqlCapabilities.java | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/null.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/null.csv-spec index 215f7782939d2..02108cf535058 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/null.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/null.csv-spec @@ -205,6 +205,7 @@ fullname:keyword | job_positions:keyword | salary:integer | salary_change:double ; inConvertedNull +required_capability: filter_in_converted_null FROM employees | WHERE emp_no in (10021, 10022, null::int) | KEEP emp_no, first_name, last_name diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java index 18fe50678040f..33dd99db83c3d 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java @@ -949,6 +949,12 @@ public enum Cap { */ FIX_REPLACE_MISSING_FIELD_WITH_NULL_DUPLICATE_NAME_ID_IN_LAYOUT, + /** + * Support for filter in converted null. + * See ESQL: Fix `NULL` handling in `IN` clause #125832 + */ + FILTER_IN_CONVERTED_NULL, + /** * When creating constant null blocks in {@link org.elasticsearch.compute.lucene.ValuesSourceReaderOperator}, we also handed off * the ownership of that block - but didn't account for the fact that the caller might close it, leading to double releases From 9e4100a6a6c4fd1ac3a689ba8252f7f054060b89 Mon Sep 17 00:00:00 2001 From: kanoshiou Date: Fri, 11 Apr 2025 17:31:25 +0800 Subject: [PATCH 6/6] Update tests --- .../esql/qa/testFixtures/src/main/resources/null.csv-spec | 1 + .../esql/expression/predicate/operator/comparison/InTests.java | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/null.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/null.csv-spec index 02108cf535058..ccf8dde563f37 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/null.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/null.csv-spec @@ -209,6 +209,7 @@ required_capability: filter_in_converted_null FROM employees | WHERE emp_no in (10021, 10022, null::int) | KEEP emp_no, first_name, last_name +| SORT emp_no ; emp_no:integer | first_name:keyword | last_name:keyword diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InTests.java index 8def801de4325..e26b6ae616865 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InTests.java @@ -91,7 +91,7 @@ public void testConvertedNull() { In in = new In( EMPTY, new FieldAttribute(Source.EMPTY, "field", new EsField("suffix", DataType.KEYWORD, Map.of(), true)), - Arrays.asList(ONE, new Literal(Source.EMPTY, null, DataType.INTEGER), THREE) + Arrays.asList(ONE, new Literal(Source.EMPTY, null, randomFrom(DataType.types())), THREE) ); var query = in.asQuery(TranslatorHandler.TRANSLATOR_HANDLER); assertEquals(new TermsQuery(EMPTY, "field", Set.of(1, 3)), query);