Skip to content

Commit 06c5edc

Browse files
costincostin
committed
SQL: Correct error message (#30138)
Error messages had placeholders that were not replaced; this PR fixes that Fix #30016
1 parent a04adaa commit 06c5edc

File tree

5 files changed

+70
-14
lines changed

5 files changed

+70
-14
lines changed

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import org.elasticsearch.xpack.sql.expression.BinaryExpression;
1111
import org.elasticsearch.xpack.sql.expression.Expression;
1212
import org.elasticsearch.xpack.sql.expression.ExpressionId;
13+
import org.elasticsearch.xpack.sql.expression.Expressions;
1314
import org.elasticsearch.xpack.sql.expression.FieldAttribute;
1415
import org.elasticsearch.xpack.sql.expression.Literal;
1516
import org.elasticsearch.xpack.sql.expression.NamedExpression;
@@ -159,7 +160,7 @@ static QueryTranslation toQuery(Expression e, boolean onAggs) {
159160
}
160161
}
161162

162-
throw new UnsupportedOperationException(format(Locale.ROOT, "Don't know how to translate %s %s", e.nodeName(), e));
163+
throw new SqlIllegalArgumentException("Don't know how to translate {} {}", e.nodeName(), e);
163164
}
164165

165166
static LeafAgg toAgg(String id, Function f) {
@@ -171,7 +172,7 @@ static LeafAgg toAgg(String id, Function f) {
171172
}
172173
}
173174

174-
throw new UnsupportedOperationException(format(Locale.ROOT, "Don't know how to translate %s %s", f.nodeName(), f));
175+
throw new SqlIllegalArgumentException("Don't know how to translate {} {}", f.nodeName(), f);
175176
}
176177

177178
static class GroupingContext {
@@ -395,8 +396,8 @@ static String field(AggregateFunction af) {
395396
if (arg instanceof Literal) {
396397
return String.valueOf(((Literal) arg).value());
397398
}
398-
throw new SqlIllegalArgumentException("Does not know how to convert argument " + arg.nodeString()
399-
+ " for function " + af.nodeString());
399+
throw new SqlIllegalArgumentException("Does not know how to convert argument {} for function {}", arg.nodeString(),
400+
af.nodeString());
400401
}
401402

402403
// TODO: need to optimize on ngram
@@ -505,9 +506,9 @@ static class BinaryComparisons extends ExpressionTranslator<BinaryComparison> {
505506
@Override
506507
protected QueryTranslation asQuery(BinaryComparison bc, boolean onAggs) {
507508
Check.isTrue(bc.right().foldable(),
508-
"Line %d:%d - Comparisons against variables are not (currently) supported; offender %s in %s",
509+
"Line {}:{}: Comparisons against variables are not (currently) supported; offender [{}] in [{}]",
509510
bc.right().location().getLineNumber(), bc.right().location().getColumnNumber(),
510-
bc.right().nodeName(), bc.nodeName());
511+
Expressions.name(bc.right()), bc.symbol());
511512

512513
if (bc.left() instanceof NamedExpression) {
513514
NamedExpression ne = (NamedExpression) bc.left();
@@ -605,8 +606,8 @@ private static Query translateQuery(BinaryComparison bc) {
605606
return new TermQuery(loc, name, value);
606607
}
607608

608-
Check.isTrue(false, "don't know how to translate binary comparison [{}] in [{}]", bc.right().nodeString(), bc);
609-
return null;
609+
throw new SqlIllegalArgumentException("Don't know how to translate binary comparison [{}] in [{}]", bc.right().nodeString(),
610+
bc);
610611
}
611612
}
612613

@@ -700,9 +701,8 @@ else if (onAggs) {
700701
return new QueryTranslation(query, aggFilter);
701702
}
702703
else {
703-
throw new UnsupportedOperationException("No idea how to translate " + e);
704+
throw new SqlIllegalArgumentException("No idea how to translate " + e);
704705
}
705-
706706
}
707707
}
708708

‎x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/FieldAttributeTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ public void testDottedFieldPathTypo() {
153153
public void testStarExpansionExcludesObjectAndUnsupportedTypes() {
154154
LogicalPlan plan = plan("SELECT * FROM test");
155155
List<? extends NamedExpression> list = ((Project) plan).projections();
156-
assertThat(list, hasSize(7));
156+
assertThat(list, hasSize(8));
157157
List<String> names = Expressions.names(list);
158158
assertThat(names, not(hasItem("some")));
159159
assertThat(names, not(hasItem("some.dotted")));

‎x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public class SysColumnsTests extends ESTestCase {
1717
public void testSysColumns() {
1818
List<List<?>> rows = new ArrayList<>();
1919
SysColumns.fillInRows("test", "index", TypesTests.loadMapping("mapping-multi-field-variation.json", true), null, rows, null);
20-
assertEquals(15, rows.size());
20+
assertEquals(16, rows.size());
2121
assertEquals(24, rows.get(0).size());
2222

2323
List<?> row = rows.get(0);
@@ -38,13 +38,13 @@ public void testSysColumns() {
3838
assertEquals(null, radix(row));
3939
assertEquals(Integer.MAX_VALUE, bufferLength(row));
4040

41-
row = rows.get(6);
41+
row = rows.get(7);
4242
assertEquals("some.dotted", name(row));
4343
assertEquals(Types.STRUCT, sqlType(row));
4444
assertEquals(null, radix(row));
4545
assertEquals(-1, bufferLength(row));
4646

47-
row = rows.get(14);
47+
row = rows.get(15);
4848
assertEquals("some.ambiguous.normalized", name(row));
4949
assertEquals(Types.VARCHAR, sqlType(row));
5050
assertEquals(null, radix(row));

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

Lines changed: 55 additions & 0 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.test.ESTestCase;
9+
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
910
import org.elasticsearch.xpack.sql.analysis.analyzer.Analyzer;
1011
import org.elasticsearch.xpack.sql.analysis.index.EsIndex;
1112
import org.elasticsearch.xpack.sql.analysis.index.IndexResolution;
@@ -18,9 +19,11 @@
1819
import org.elasticsearch.xpack.sql.plan.logical.Project;
1920
import org.elasticsearch.xpack.sql.planner.QueryTranslator.QueryTranslation;
2021
import org.elasticsearch.xpack.sql.querydsl.query.Query;
22+
import org.elasticsearch.xpack.sql.querydsl.query.RangeQuery;
2123
import org.elasticsearch.xpack.sql.querydsl.query.TermQuery;
2224
import org.elasticsearch.xpack.sql.type.EsField;
2325
import org.elasticsearch.xpack.sql.type.TypesTests;
26+
import org.joda.time.DateTime;
2427

2528
import java.util.Map;
2629
import java.util.TimeZone;
@@ -84,4 +87,56 @@ public void testTermEqualityNotAnalyzed() {
8487
assertEquals("int", tq.term());
8588
assertEquals(5, tq.value());
8689
}
90+
91+
public void testComparisonAgainstColumns() {
92+
LogicalPlan p = plan("SELECT some.string FROM test WHERE date > int");
93+
assertTrue(p instanceof Project);
94+
p = ((Project) p).child();
95+
assertTrue(p instanceof Filter);
96+
Expression condition = ((Filter) p).condition();
97+
SqlIllegalArgumentException ex = expectThrows(SqlIllegalArgumentException.class, () -> QueryTranslator.toQuery(condition, false));
98+
assertEquals("Line 1:43: Comparisons against variables are not (currently) supported; offender [int] in [>]", ex.getMessage());
99+
}
100+
101+
public void testDateRange() {
102+
LogicalPlan p = plan("SELECT some.string FROM test WHERE date > 1969-05-13");
103+
assertTrue(p instanceof Project);
104+
p = ((Project) p).child();
105+
assertTrue(p instanceof Filter);
106+
Expression condition = ((Filter) p).condition();
107+
QueryTranslation translation = QueryTranslator.toQuery(condition, false);
108+
Query query = translation.query;
109+
assertTrue(query instanceof RangeQuery);
110+
RangeQuery rq = (RangeQuery) query;
111+
assertEquals("date", rq.field());
112+
assertEquals(1951, rq.lower());
113+
}
114+
115+
public void testDateRangeLiteral() {
116+
LogicalPlan p = plan("SELECT some.string FROM test WHERE date > '1969-05-13'");
117+
assertTrue(p instanceof Project);
118+
p = ((Project) p).child();
119+
assertTrue(p instanceof Filter);
120+
Expression condition = ((Filter) p).condition();
121+
QueryTranslation translation = QueryTranslator.toQuery(condition, false);
122+
Query query = translation.query;
123+
assertTrue(query instanceof RangeQuery);
124+
RangeQuery rq = (RangeQuery) query;
125+
assertEquals("date", rq.field());
126+
assertEquals("1969-05-13", rq.lower());
127+
}
128+
129+
public void testDateRangeCast() {
130+
LogicalPlan p = plan("SELECT some.string FROM test WHERE date > CAST('1969-05-13T12:34:56Z' AS DATE)");
131+
assertTrue(p instanceof Project);
132+
p = ((Project) p).child();
133+
assertTrue(p instanceof Filter);
134+
Expression condition = ((Filter) p).condition();
135+
QueryTranslation translation = QueryTranslator.toQuery(condition, false);
136+
Query query = translation.query;
137+
assertTrue(query instanceof RangeQuery);
138+
RangeQuery rq = (RangeQuery) query;
139+
assertEquals("date", rq.field());
140+
assertEquals(DateTime.parse("1969-05-13T12:34:56Z"), rq.lower());
141+
}
87142
}

‎x-pack/plugin/sql/src/test/resources/mapping-multi-field-variation.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
"int" : { "type" : "integer" },
55
"text" : { "type" : "text" },
66
"keyword" : { "type" : "keyword" },
7+
"date" : { "type" : "date" },
78
"unsupported" : { "type" : "ip_range" },
89
"some" : {
910
"properties" : {

0 commit comments

Comments
 (0)