Skip to content

Commit f04f19e

Browse files
Marios Trivyzascostin
authored and
matriv
committed
SQL: Fix queries with filter resulting in NO_MATCH (#34812)
Previously, `Mapper` was returning an incorrect plan which resulted in an ``` SQLFeatureNotSupportedException: Found 1 problem(s) line 1:8: Unexecutable item ``` Queries with a `WHERE` and/or `HAVING` clause which results in NO_MATCH are now handled correctly and return 0 rows. Fixes: #34613 Co-authored-by: Costin Leau <[email protected]>
1 parent fcad4e7 commit f04f19e

File tree

5 files changed

+113
-3
lines changed

5 files changed

+113
-3
lines changed

‎x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/Mapper.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ protected PhysicalPlan map(LogicalPlan p) {
6464
}
6565

6666
if (p instanceof LocalRelation) {
67-
return new LocalExec(p.location(), (LocalRelation) p);
67+
return new LocalExec(p.location(), ((LocalRelation) p).executable());
6868
}
6969

7070
if (p instanceof Project) {

‎x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryFolder.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package org.elasticsearch.xpack.sql.planner;
77

88
import org.elasticsearch.common.collect.Tuple;
9+
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
910
import org.elasticsearch.xpack.sql.execution.search.AggRef;
1011
import org.elasticsearch.xpack.sql.expression.Alias;
1112
import org.elasticsearch.xpack.sql.expression.Attribute;
@@ -525,8 +526,12 @@ private static class PropagateEmptyLocal extends FoldingRule<PhysicalPlan> {
525526
protected PhysicalPlan rule(PhysicalPlan plan) {
526527
if (plan.children().size() == 1) {
527528
PhysicalPlan p = plan.children().get(0);
528-
if (p instanceof LocalExec && ((LocalExec) p).isEmpty()) {
529-
return new LocalExec(plan.location(), new EmptyExecutable(plan.output()));
529+
if (p instanceof LocalExec) {
530+
if (((LocalExec) p).isEmpty()) {
531+
return new LocalExec(plan.location(), new EmptyExecutable(plan.output()));
532+
} else {
533+
throw new SqlIllegalArgumentException("Encountered a bug; {} is a LocalExec but is not empty", p);
534+
}
530535
}
531536
}
532537
return plan;

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

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
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+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
package org.elasticsearch.xpack.sql.planner;
7+
8+
import org.elasticsearch.test.AbstractBuilderTestCase;
9+
import org.elasticsearch.xpack.sql.analysis.analyzer.Analyzer;
10+
import org.elasticsearch.xpack.sql.analysis.index.EsIndex;
11+
import org.elasticsearch.xpack.sql.analysis.index.IndexResolution;
12+
import org.elasticsearch.xpack.sql.expression.function.FunctionRegistry;
13+
import org.elasticsearch.xpack.sql.optimizer.Optimizer;
14+
import org.elasticsearch.xpack.sql.parser.SqlParser;
15+
import org.elasticsearch.xpack.sql.plan.physical.LocalExec;
16+
import org.elasticsearch.xpack.sql.plan.physical.PhysicalPlan;
17+
import org.elasticsearch.xpack.sql.session.EmptyExecutable;
18+
import org.elasticsearch.xpack.sql.type.EsField;
19+
import org.elasticsearch.xpack.sql.type.TypesTests;
20+
import org.junit.AfterClass;
21+
import org.junit.BeforeClass;
22+
23+
import java.util.Map;
24+
import java.util.TimeZone;
25+
26+
import static org.hamcrest.Matchers.startsWith;
27+
28+
public class QueryFolderTests extends AbstractBuilderTestCase {
29+
30+
private static SqlParser parser;
31+
private static Analyzer analyzer;
32+
private static Optimizer optimizer;
33+
private static Planner planner;
34+
35+
@BeforeClass
36+
public static void init() {
37+
parser = new SqlParser();
38+
39+
Map<String, EsField> mapping = TypesTests.loadMapping("mapping-multi-field-variation.json");
40+
EsIndex test = new EsIndex("test", mapping);
41+
IndexResolution getIndexResult = IndexResolution.valid(test);
42+
analyzer = new Analyzer(new FunctionRegistry(), getIndexResult, TimeZone.getTimeZone("UTC"));
43+
optimizer = new Optimizer();
44+
planner = new Planner();
45+
}
46+
47+
@AfterClass
48+
public static void destroy() {
49+
parser = null;
50+
analyzer = null;
51+
}
52+
53+
private PhysicalPlan plan(String sql) {
54+
return planner.plan(optimizer.optimize(analyzer.analyze(parser.createStatement(sql), true)), true);
55+
}
56+
57+
public void testFoldingToLocalExecWithProject() {
58+
PhysicalPlan p = plan("SELECT keyword FROM test WHERE 1 = 2");
59+
assertEquals(LocalExec.class, p.getClass());
60+
LocalExec le = (LocalExec) p;
61+
assertEquals(EmptyExecutable.class, le.executable().getClass());
62+
EmptyExecutable ee = (EmptyExecutable) le.executable();
63+
assertEquals(1, ee.output().size());
64+
assertThat(ee.output().get(0).toString(), startsWith("keyword{f}#"));
65+
}
66+
67+
public void testFoldingToLocalExecWithProject_WithOrderAndLimit() {
68+
PhysicalPlan p = plan("SELECT keyword FROM test WHERE 1 = 2 ORDER BY int LIMIT 10");
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+
77+
public void testFoldingToLocalExecWithProjectWithGroupBy_WithOrderAndLimit() {
78+
PhysicalPlan p = plan("SELECT keyword, max(int) FROM test WHERE 1 = 2 GROUP BY keyword ORDER BY 1 LIMIT 10");
79+
assertEquals(LocalExec.class, p.getClass());
80+
LocalExec le = (LocalExec) p;
81+
assertEquals(EmptyExecutable.class, le.executable().getClass());
82+
EmptyExecutable ee = (EmptyExecutable) le.executable();
83+
assertEquals(2, ee.output().size());
84+
assertThat(ee.output().get(0).toString(), startsWith("keyword{f}#"));
85+
assertThat(ee.output().get(1).toString(), startsWith("MAX(int){a->"));
86+
}
87+
88+
public void testFoldingToLocalExecWithProjectWithGroupBy_WithHaving_WithOrderAndLimit() {
89+
PhysicalPlan p = plan("SELECT keyword, max(int) FROM test GROUP BY keyword HAVING 1 = 2 ORDER BY 1 LIMIT 10");
90+
assertEquals(LocalExec.class, p.getClass());
91+
LocalExec le = (LocalExec) p;
92+
assertEquals(EmptyExecutable.class, le.executable().getClass());
93+
EmptyExecutable ee = (EmptyExecutable) le.executable();
94+
assertEquals(2, ee.output().size());
95+
assertThat(ee.output().get(0).toString(), startsWith("keyword{f}#"));
96+
assertThat(ee.output().get(1).toString(), startsWith("MAX(int){a->"));
97+
}
98+
}

‎x-pack/qa/sql/src/main/resources/agg.sql-spec‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,10 @@ aggMultiGroupByMultiWithHavingUsingIn
435435
SELECT MIN(salary) min, MAX(salary) max, gender g, languages l, COUNT(*) c FROM "test_emp" WHERE languages > 0 GROUP BY g, languages HAVING max IN (74500, 74600) ORDER BY gender, languages;
436436

437437

438+
// HAVING filter resulting in NoMatch
439+
aggWithNoMatchHaving
440+
SELECT gender g, COUNT(*) c FROM "test_emp" GROUP BY g HAVING 1 > 2 ORDER BY gender;
441+
438442
//
439443
// NULL tests
440444
//

‎x-pack/qa/sql/src/main/resources/filter.sql-spec‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,9 @@ SELECT last_name l FROM "test_emp" WHERE emp_no BETWEEN 9990 AND 10003 ORDER BY
7979
whereNotBetween
8080
SELECT last_name l FROM "test_emp" WHERE emp_no NOT BETWEEN 10010 AND 10020 ORDER BY emp_no LIMIT 5;
8181

82+
whereNoMatch
83+
SELECT last_name l FROM "test_emp" WHERE 1 = 2 ORDER BY 1 LIMIT 10;
84+
8285
//
8386
// IN expression
8487
//

0 commit comments

Comments
 (0)