Skip to content

Commit d71a718

Browse files
luigidellaquilaluigidellaquila
committed
SQL: Fix QlIllegalArgumentException with non-foldable date range queries (elastic#142386)
1 parent 149178c commit d71a718

File tree

5 files changed

+155
-1
lines changed

5 files changed

+155
-1
lines changed

‎docs/changelog/142386.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
area: SQL
2+
issues:
3+
- 137365
4+
pr: 142386
5+
summary: Fix `QlIllegalArgumentException` with non-foldable date range queries
6+
type: bug

‎x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/ExpressionTranslators.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,13 @@ protected Query asQuery(Range r, TranslatorHandler handler) {
364364
}
365365

366366
public static Query doTranslate(Range r, TranslatorHandler handler) {
367-
return handler.wrapFunctionQuery(r, r.value(), () -> translate(r, handler));
367+
// Check if bounds are foldable for native RangeQuery
368+
if (r.lower().foldable() && r.upper().foldable()) {
369+
return handler.wrapFunctionQuery(r, r.value(), () -> translate(r, handler));
370+
}
371+
// Fall back to script query for non-foldable bounds (e.g., DATE_ADD with field reference)
372+
Query q = new ScriptQuery(r.source(), r.asScript());
373+
return wrapIfNested(q, r.value());
368374
}
369375

370376
private static RangeQuery translate(Range r, TranslatorHandler handler) {

‎x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/planner/ExpressionTranslatorsTests.java‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
package org.elasticsearch.xpack.ql.planner;
8+
9+
import org.elasticsearch.test.ESTestCase;
10+
import org.elasticsearch.xpack.ql.expression.Expression;
11+
import org.elasticsearch.xpack.ql.expression.FieldAttribute;
12+
import org.elasticsearch.xpack.ql.expression.Literal;
13+
import org.elasticsearch.xpack.ql.expression.predicate.Range;
14+
import org.elasticsearch.xpack.ql.querydsl.query.NestedQuery;
15+
import org.elasticsearch.xpack.ql.querydsl.query.Query;
16+
import org.elasticsearch.xpack.ql.querydsl.query.ScriptQuery;
17+
import org.elasticsearch.xpack.ql.tree.Source;
18+
import org.elasticsearch.xpack.ql.type.DataTypes;
19+
import org.elasticsearch.xpack.ql.type.EsField;
20+
21+
import java.time.ZoneOffset;
22+
import java.time.ZonedDateTime;
23+
import java.util.Collections;
24+
25+
import static org.hamcrest.Matchers.containsString;
26+
import static org.hamcrest.Matchers.instanceOf;
27+
28+
public class ExpressionTranslatorsTests extends ESTestCase {
29+
30+
private static final Source SOURCE = Source.EMPTY;
31+
32+
public void testRangeWithNonFoldableBoundsOnNestedFieldReturnsNestedQuery() {
33+
EsField nestedEsField = new EsField("nested", DataTypes.NESTED, Collections.emptyMap(), false);
34+
FieldAttribute nestedParent = new FieldAttribute(SOURCE, "nested", nestedEsField);
35+
36+
EsField dateEsField = new EsField("date", DataTypes.DATETIME, Collections.emptyMap(), true);
37+
FieldAttribute nestedDateField = new FieldAttribute(SOURCE, nestedParent, "nested.date", dateEsField);
38+
39+
assertTrue(nestedDateField.isNested());
40+
assertEquals("nested", nestedDateField.nestedParent().name());
41+
42+
// Use the field itself as lower bound to make it non-foldable
43+
Expression nonFoldableLower = nestedDateField;
44+
Expression foldableUpper = new Literal(SOURCE, ZonedDateTime.now(ZoneOffset.UTC), DataTypes.DATETIME);
45+
46+
Range range = new Range(SOURCE, nestedDateField, nonFoldableLower, true, foldableUpper, true, ZoneOffset.UTC);
47+
assertFalse(range.lower().foldable());
48+
assertTrue(range.upper().foldable());
49+
50+
Query result = ExpressionTranslators.Ranges.doTranslate(range, new QlTranslatorHandler());
51+
52+
assertThat(result, instanceOf(NestedQuery.class));
53+
String queryString = result.asBuilder().toString();
54+
assertThat(queryString, containsString("\"nested\""));
55+
assertThat(queryString, containsString("\"script\""));
56+
}
57+
58+
public void testRangeWithNonFoldableBoundsOnNonNestedFieldReturnsScriptQuery() {
59+
EsField dateEsField = new EsField("date", DataTypes.DATETIME, Collections.emptyMap(), true);
60+
FieldAttribute dateField = new FieldAttribute(SOURCE, "date", dateEsField);
61+
62+
assertFalse(dateField.isNested());
63+
64+
Expression nonFoldableLower = dateField;
65+
Expression foldableUpper = new Literal(SOURCE, ZonedDateTime.now(ZoneOffset.UTC), DataTypes.DATETIME);
66+
67+
Range range = new Range(SOURCE, dateField, nonFoldableLower, true, foldableUpper, true, ZoneOffset.UTC);
68+
69+
Query result = ExpressionTranslators.Ranges.doTranslate(range, new QlTranslatorHandler());
70+
71+
assertThat(result, instanceOf(ScriptQuery.class));
72+
}
73+
}

‎x-pack/plugin/sql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/sql/qa/single_node/RestSqlIT.java‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,11 @@
1515
import org.junit.ClassRule;
1616

1717
import java.io.IOException;
18+
import java.util.List;
19+
import java.util.Map;
1820

1921
import static org.hamcrest.Matchers.containsString;
22+
import static org.hamcrest.Matchers.hasSize;
2023

2124
/**
2225
* Integration test for the rest sql action. The one that speaks json directly to a
@@ -97,4 +100,53 @@ public void testIncorrectAcceptHeader() throws IOException {
97100
containsString("Invalid request content type: Accept=[application/fff]")
98101
);
99102
}
103+
104+
/**
105+
* Verifies the fix for https://github.com/elastic/elasticsearch/issues/137365
106+
* DATE_ADD with a field reference in a range bound should work via script query.
107+
*/
108+
@SuppressWarnings("unchecked")
109+
public void testDateAddWithFieldInRangeBound() throws IOException {
110+
// Create index with date fields
111+
Request createIndex = new Request("PUT", "/test_date_add");
112+
createIndex.setJsonEntity("""
113+
{
114+
"mappings": {
115+
"properties": {
116+
"event_date": { "type": "date", "format": "yyyy-MM-dd" },
117+
"base_date": { "type": "date", "format": "yyyy-MM-dd" },
118+
"name": { "type": "keyword" }
119+
}
120+
}
121+
}""");
122+
provisioningClient().performRequest(createIndex);
123+
124+
// Index test documents
125+
Request bulk = new Request("POST", "/test_date_add/_bulk");
126+
bulk.addParameter("refresh", "true");
127+
bulk.setJsonEntity("""
128+
{"index":{}}
129+
{"event_date":"2024-01-15","base_date":"2024-01-05","name":"within_range"}
130+
{"index":{}}
131+
{"event_date":"2024-03-05","base_date":"2024-01-05","name":"outside_range"}
132+
{"index":{}}
133+
{"event_date":"2024-02-01","base_date":"2024-02-01","name":"same_date"}
134+
""");
135+
provisioningClient().performRequest(bulk);
136+
137+
// Query: find documents where event_date is within 30 days after base_date
138+
// This uses DATE_ADD with a field reference, which requires script query fallback
139+
String mode = randomMode();
140+
String sql = "SELECT name FROM test_date_add "
141+
+ "WHERE event_date BETWEEN base_date AND DATE_ADD('days', 30, base_date) ORDER BY name";
142+
Map<String, Object> result = runSql(new StringEntity(query(sql).mode(mode).toString(), ContentType.APPLICATION_JSON), "", mode);
143+
144+
List<List<Object>> rows = (List<List<Object>>) result.get("rows");
145+
assertThat(rows, hasSize(2));
146+
assertEquals("same_date", rows.get(0).get(0));
147+
assertEquals("within_range", rows.get(1).get(0));
148+
149+
// Cleanup
150+
provisioningClient().performRequest(new Request("DELETE", "/test_date_add"));
151+
}
100152
}

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,23 @@ public void testFoldingToLocalExecWithProjectAndOrderByAndLimit() {
114114
assertThat(ee.output().get(0).toString(), startsWith("test.keyword{f}#"));
115115
}
116116

117+
/**
118+
* Verifies the fix for https://github.com/elastic/elasticsearch/issues/137365
119+
* DATE_ADD with a field as the third argument is not foldable; when used as a range bound
120+
* the planner falls back to a script query instead of throwing an exception.
121+
*/
122+
public void testDateAddWithFieldInRangeBoundUsesScriptQuery() {
123+
// WHERE date BETWEEN DATE_ADD('days', -30, date) AND CURRENT_TIMESTAMP
124+
// produces a Range with non-foldable lower bound, which should fall back to a script query
125+
PhysicalPlan p = plan("SELECT * FROM test WHERE date BETWEEN DATE_ADD('days', -30, date) AND CURRENT_TIMESTAMP");
126+
assertEquals(EsQueryExec.class, p.getClass());
127+
EsQueryExec ee = (EsQueryExec) p;
128+
String query = ee.queryContainer().query().asBuilder().toString().replaceAll("\\s+", "");
129+
// Verify it generates a script query with the dateAdd function
130+
assertThat(query, containsString("\"script\""));
131+
assertThat(query, containsString("InternalSqlScriptUtils.dateAdd"));
132+
}
133+
117134
public void testLocalExecWithPrunedFilterWithFunction() {
118135
PhysicalPlan p = plan("SELECT E() FROM test WHERE PI() = 5");
119136
assertEquals(LocalExec.class, p.getClass());

0 commit comments

Comments
 (0)