diff --git a/docs/changelog/142386.yaml b/docs/changelog/142386.yaml new file mode 100644 index 0000000000000..6ca4403acfb08 --- /dev/null +++ b/docs/changelog/142386.yaml @@ -0,0 +1,6 @@ +area: SQL +issues: + - 137365 +pr: 142386 +summary: Fix `QlIllegalArgumentException` with non-foldable date range queries +type: bug diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/ExpressionTranslators.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/ExpressionTranslators.java index e8c9e3aead362..5762f49f7d438 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/ExpressionTranslators.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/ExpressionTranslators.java @@ -364,7 +364,13 @@ protected Query asQuery(Range r, TranslatorHandler handler) { } public static Query doTranslate(Range r, TranslatorHandler handler) { - return handler.wrapFunctionQuery(r, r.value(), () -> translate(r, handler)); + // Check if bounds are foldable for native RangeQuery + if (r.lower().foldable() && r.upper().foldable()) { + return handler.wrapFunctionQuery(r, r.value(), () -> translate(r, handler)); + } + // Fall back to script query for non-foldable bounds (e.g., DATE_ADD with field reference) + Query q = new ScriptQuery(r.source(), r.asScript()); + return wrapIfNested(q, r.value()); } private static RangeQuery translate(Range r, TranslatorHandler handler) { diff --git a/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/planner/ExpressionTranslatorsTests.java b/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/planner/ExpressionTranslatorsTests.java new file mode 100644 index 0000000000000..4ae85f3f1be00 --- /dev/null +++ b/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/planner/ExpressionTranslatorsTests.java @@ -0,0 +1,73 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ +package org.elasticsearch.xpack.ql.planner; + +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.ql.expression.Expression; +import org.elasticsearch.xpack.ql.expression.FieldAttribute; +import org.elasticsearch.xpack.ql.expression.Literal; +import org.elasticsearch.xpack.ql.expression.predicate.Range; +import org.elasticsearch.xpack.ql.querydsl.query.NestedQuery; +import org.elasticsearch.xpack.ql.querydsl.query.Query; +import org.elasticsearch.xpack.ql.querydsl.query.ScriptQuery; +import org.elasticsearch.xpack.ql.tree.Source; +import org.elasticsearch.xpack.ql.type.DataTypes; +import org.elasticsearch.xpack.ql.type.EsField; + +import java.time.ZoneOffset; +import java.time.ZonedDateTime; +import java.util.Collections; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.instanceOf; + +public class ExpressionTranslatorsTests extends ESTestCase { + + private static final Source SOURCE = Source.EMPTY; + + public void testRangeWithNonFoldableBoundsOnNestedFieldReturnsNestedQuery() { + EsField nestedEsField = new EsField("nested", DataTypes.NESTED, Collections.emptyMap(), false); + FieldAttribute nestedParent = new FieldAttribute(SOURCE, "nested", nestedEsField); + + EsField dateEsField = new EsField("date", DataTypes.DATETIME, Collections.emptyMap(), true); + FieldAttribute nestedDateField = new FieldAttribute(SOURCE, nestedParent, "nested.date", dateEsField); + + assertTrue(nestedDateField.isNested()); + assertEquals("nested", nestedDateField.nestedParent().name()); + + // Use the field itself as lower bound to make it non-foldable + Expression nonFoldableLower = nestedDateField; + Expression foldableUpper = new Literal(SOURCE, ZonedDateTime.now(ZoneOffset.UTC), DataTypes.DATETIME); + + Range range = new Range(SOURCE, nestedDateField, nonFoldableLower, true, foldableUpper, true, ZoneOffset.UTC); + assertFalse(range.lower().foldable()); + assertTrue(range.upper().foldable()); + + Query result = ExpressionTranslators.Ranges.doTranslate(range, new QlTranslatorHandler()); + + assertThat(result, instanceOf(NestedQuery.class)); + String queryString = result.asBuilder().toString(); + assertThat(queryString, containsString("\"nested\"")); + assertThat(queryString, containsString("\"script\"")); + } + + public void testRangeWithNonFoldableBoundsOnNonNestedFieldReturnsScriptQuery() { + EsField dateEsField = new EsField("date", DataTypes.DATETIME, Collections.emptyMap(), true); + FieldAttribute dateField = new FieldAttribute(SOURCE, "date", dateEsField); + + assertFalse(dateField.isNested()); + + Expression nonFoldableLower = dateField; + Expression foldableUpper = new Literal(SOURCE, ZonedDateTime.now(ZoneOffset.UTC), DataTypes.DATETIME); + + Range range = new Range(SOURCE, dateField, nonFoldableLower, true, foldableUpper, true, ZoneOffset.UTC); + + Query result = ExpressionTranslators.Ranges.doTranslate(range, new QlTranslatorHandler()); + + assertThat(result, instanceOf(ScriptQuery.class)); + } +} diff --git a/x-pack/plugin/sql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/sql/qa/single_node/RestSqlIT.java b/x-pack/plugin/sql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/sql/qa/single_node/RestSqlIT.java index 167cc212685d7..63b98df5c7db3 100644 --- a/x-pack/plugin/sql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/sql/qa/single_node/RestSqlIT.java +++ b/x-pack/plugin/sql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/sql/qa/single_node/RestSqlIT.java @@ -15,8 +15,11 @@ import org.junit.ClassRule; import java.io.IOException; +import java.util.List; +import java.util.Map; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.hasSize; /** * Integration test for the rest sql action. The one that speaks json directly to a @@ -97,4 +100,53 @@ public void testIncorrectAcceptHeader() throws IOException { containsString("Invalid request content type: Accept=[application/fff]") ); } + + /** + * Verifies the fix for https://github.com/elastic/elasticsearch/issues/137365 + * DATE_ADD with a field reference in a range bound should work via script query. + */ + @SuppressWarnings("unchecked") + public void testDateAddWithFieldInRangeBound() throws IOException { + // Create index with date fields + Request createIndex = new Request("PUT", "/test_date_add"); + createIndex.setJsonEntity(""" + { + "mappings": { + "properties": { + "event_date": { "type": "date", "format": "yyyy-MM-dd" }, + "base_date": { "type": "date", "format": "yyyy-MM-dd" }, + "name": { "type": "keyword" } + } + } + }"""); + provisioningClient().performRequest(createIndex); + + // Index test documents + Request bulk = new Request("POST", "/test_date_add/_bulk"); + bulk.addParameter("refresh", "true"); + bulk.setJsonEntity(""" + {"index":{}} + {"event_date":"2024-01-15","base_date":"2024-01-05","name":"within_range"} + {"index":{}} + {"event_date":"2024-03-05","base_date":"2024-01-05","name":"outside_range"} + {"index":{}} + {"event_date":"2024-02-01","base_date":"2024-02-01","name":"same_date"} + """); + provisioningClient().performRequest(bulk); + + // Query: find documents where event_date is within 30 days after base_date + // This uses DATE_ADD with a field reference, which requires script query fallback + String mode = randomMode(); + String sql = "SELECT name FROM test_date_add " + + "WHERE event_date BETWEEN base_date AND DATE_ADD('days', 30, base_date) ORDER BY name"; + Map result = runSql(new StringEntity(query(sql).mode(mode).toString(), ContentType.APPLICATION_JSON), "", mode); + + List> rows = (List>) result.get("rows"); + assertThat(rows, hasSize(2)); + assertEquals("same_date", rows.get(0).get(0)); + assertEquals("within_range", rows.get(1).get(0)); + + // Cleanup + provisioningClient().performRequest(new Request("DELETE", "/test_date_add")); + } } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java index 72d5fc2864653..7eb13ea811c26 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java @@ -114,6 +114,23 @@ public void testFoldingToLocalExecWithProjectAndOrderByAndLimit() { assertThat(ee.output().get(0).toString(), startsWith("test.keyword{f}#")); } + /** + * Verifies the fix for https://github.com/elastic/elasticsearch/issues/137365 + * DATE_ADD with a field as the third argument is not foldable; when used as a range bound + * the planner falls back to a script query instead of throwing an exception. + */ + public void testDateAddWithFieldInRangeBoundUsesScriptQuery() { + // WHERE date BETWEEN DATE_ADD('days', -30, date) AND CURRENT_TIMESTAMP + // produces a Range with non-foldable lower bound, which should fall back to a script query + PhysicalPlan p = plan("SELECT * FROM test WHERE date BETWEEN DATE_ADD('days', -30, date) AND CURRENT_TIMESTAMP"); + assertEquals(EsQueryExec.class, p.getClass()); + EsQueryExec ee = (EsQueryExec) p; + String query = ee.queryContainer().query().asBuilder().toString().replaceAll("\\s+", ""); + // Verify it generates a script query with the dateAdd function + assertThat(query, containsString("\"script\"")); + assertThat(query, containsString("InternalSqlScriptUtils.dateAdd")); + } + public void testLocalExecWithPrunedFilterWithFunction() { PhysicalPlan p = plan("SELECT E() FROM test WHERE PI() = 5"); assertEquals(LocalExec.class, p.getClass());