Skip to content

Commit

Permalink
Fix error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
@astefan
astefan committed Sep 12, 2024
1 parent c1f09d3 commit 40db731
Show file tree
Hide file tree
Showing 7 changed files with 125 additions and 70 deletions.
16 changes: 8 additions & 8 deletions x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1904,9 +1904,9 @@ public void testLookup() {
| RENAME languages AS int
| LOOKUP int_number_names ON int
""";
if (Build.current().isProductionRelease()) {
if (Build.current().isSnapshot() == false) {
var e = expectThrows(ParsingException.class, () -> analyze(query));
assertThat(e.getMessage(), containsString("line 3:4: LOOKUP is in preview and only available in SNAPSHOT build"));
assertThat(e.getMessage(), containsString("line 3:3: mismatched input 'LOOKUP' expecting {"));
return;
}
LogicalPlan plan = analyze(query);
Expand Down Expand Up @@ -1960,9 +1960,9 @@ public void testLookupMissingField() {
FROM test
| LOOKUP int_number_names ON garbage
""";
if (Build.current().isProductionRelease()) {
if (Build.current().isSnapshot() == false) {
var e = expectThrows(ParsingException.class, () -> analyze(query));
assertThat(e.getMessage(), containsString("line 2:4: LOOKUP is in preview and only available in SNAPSHOT build"));
assertThat(e.getMessage(), containsString("line 2:3: mismatched input 'LOOKUP' expecting {"));
return;
}
var e = expectThrows(VerificationException.class, () -> analyze(query));
Expand All @@ -1974,9 +1974,9 @@ public void testLookupMissingTable() {
FROM test
| LOOKUP garbage ON a
""";
if (Build.current().isProductionRelease()) {
if (Build.current().isSnapshot() == false) {
var e = expectThrows(ParsingException.class, () -> analyze(query));
assertThat(e.getMessage(), containsString("line 2:4: LOOKUP is in preview and only available in SNAPSHOT build"));
assertThat(e.getMessage(), containsString("line 2:3: mismatched input 'LOOKUP' expecting {"));
return;
}
var e = expectThrows(VerificationException.class, () -> analyze(query));
Expand All @@ -1989,9 +1989,9 @@ public void testLookupMatchTypeWrong() {
| RENAME last_name AS int
| LOOKUP int_number_names ON int
""";
if (Build.current().isProductionRelease()) {
if (Build.current().isSnapshot() == false) {
var e = expectThrows(ParsingException.class, () -> analyze(query));
assertThat(e.getMessage(), containsString("line 3:4: LOOKUP is in preview and only available in SNAPSHOT build"));
assertThat(e.getMessage(), containsString("line 3:3: mismatched input 'LOOKUP' expecting {"));
return;
}
var e = expectThrows(VerificationException.class, () -> analyze(query));
Expand Down
54 changes: 38 additions & 16 deletions x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.elasticsearch.xpack.esql.index.EsIndex;
import org.elasticsearch.xpack.esql.index.IndexResolution;
import org.elasticsearch.xpack.esql.parser.EsqlParser;
import org.elasticsearch.xpack.esql.parser.ParsingException;
import org.elasticsearch.xpack.esql.parser.QueryParam;
import org.elasticsearch.xpack.esql.parser.QueryParams;

Expand All @@ -35,6 +36,7 @@
import static org.elasticsearch.xpack.esql.core.type.DataType.UNSIGNED_LONG;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.matchesRegex;

//@TestLogging(value = "org.elasticsearch.xpack.esql:TRACE,org.elasticsearch.compute:TRACE", reason = "debug")
Expand Down Expand Up @@ -1075,21 +1077,36 @@ public void testMatchFilter() throws Exception {
);
}

public void testMatchCommand() throws Exception {
assumeTrue("skipping because MATCH_COMMAND is not enabled", EsqlCapabilities.Cap.MATCH_COMMAND.isEnabled());
assertEquals("1:24: MATCH cannot be used after LIMIT", error("from test | limit 10 | match \"Anna\""));
assertEquals("1:13: MATCH cannot be used after SHOW", error("show info | match \"8.16.0\""));
assertEquals("1:17: MATCH cannot be used after ROW", error("row a= \"Anna\" | match \"Anna\""));
assertEquals("1:26: MATCH cannot be used after EVAL", error("from test | eval z = 2 | match \"Anna\""));
assertEquals("1:43: MATCH cannot be used after DISSECT", error("from test | dissect first_name \"%{foo}\" | match \"Connection\""));
assertEquals("1:27: MATCH cannot be used after DROP", error("from test | drop emp_no | match \"Anna\""));
assertEquals("1:35: MATCH cannot be used after EVAL", error("from test | eval n = emp_no * 3 | match \"Anna\""));
assertEquals("1:44: MATCH cannot be used after GROK", error("from test | grok last_name \"%{WORD:foo}\" | match \"Anna\""));
assertEquals("1:27: MATCH cannot be used after KEEP", error("from test | keep emp_no | match \"Anna\""));
public void testMatchCommand() {
assertMatchCommand("1:24:", "LIMIT","from test | limit 10 | match \"Anna\"");
assertMatchCommand("1:13:", "SHOW", "show info | match \"8.16.0\"");
assertMatchCommand("1:17:", "ROW","row a= \"Anna\" | match \"Anna\"");
assertMatchCommand("1:26:", "EVAL","from test | eval z = 2 | match \"Anna\"");
assertMatchCommand("1:43:","DISSECT","from test | dissect first_name \"%{foo}\" | match \"Connection\"");
assertMatchCommand("1:27:", "DROP","from test | drop emp_no | match \"Anna\"");
assertMatchCommand("1:35:", "EVAL", "from test | eval n = emp_no * 3 | match \"Anna\"");
assertMatchCommand("1:44:", "GROK","from test | grok last_name \"%{WORD:foo}\" | match \"Anna\"");
assertMatchCommand("1:27:", "KEEP","from test | keep emp_no | match \"Anna\"");

// TODO Keep adding tests for all unsupported commands
}

private void assertMatchCommand(String lineAndColumn, String command, String query) {
String message;
Class<? extends Exception> exception;
var isSnapshot = Build.current().isSnapshot();
if (isSnapshot) {
message = " MATCH cannot be used after ";
exception = VerificationException.class;
} else {
message = " mismatched input 'match' expecting ";
exception = ParsingException.class;
}

var expectedErrorMessage = lineAndColumn + message + (isSnapshot ? command : "");
assertThat(error(query, defaultAnalyzer, exception), containsString(expectedErrorMessage));
}

public void testCoalesceWithMixedNumericTypes() {
assertEquals(
"1:22: second argument of [coalesce(languages, height)] must be [integer], found value [height] type [double]",
Expand Down Expand Up @@ -1292,6 +1309,10 @@ private String error(String query, Object... params) {
}

private String error(String query, Analyzer analyzer, Object... params) {
return error(query, analyzer, VerificationException.class, params);
}

private String error(String query, Analyzer analyzer, Class<? extends Exception> exception, Object... params) {
List<QueryParam> parameters = new ArrayList<>();
for (Object param : params) {
if (param == null) {
Expand All @@ -1304,12 +1325,13 @@ private String error(String query, Analyzer analyzer, Object... params) {
throw new IllegalArgumentException("VerifierTests don't support params of type " + param.getClass());
}
}
VerificationException e = expectThrows(
VerificationException.class,
() -> analyzer.analyze(parser.createStatement(query, new QueryParams(parameters)))
);
Throwable e = expectThrows(exception, () -> analyzer.analyze(parser.createStatement(query, new QueryParams(parameters))));
assertThat(e, instanceOf(exception));

String message = e.getMessage();
assertTrue(message.startsWith("Found "));
if (e instanceof VerificationException) {
assertTrue(message.startsWith("Found "));
}
String pattern = "\nline ";
int index = message.indexOf(pattern);
return message.substring(index + pattern.length());
Expand Down
44 changes: 26 additions & 18 deletions ...t/java/org/elasticsearch/xpack/esql/expression/function/scalar/conditional/CaseTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import com.carrotsearch.randomizedtesting.annotations.Name;
import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;

import org.elasticsearch.Build;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.esql.core.expression.Expression;
Expand All @@ -34,24 +35,31 @@
import static org.hamcrest.Matchers.startsWith;

public class CaseTests extends AbstractScalarFunctionTestCase {
private static final List<DataType> TYPES = List.of(
DataType.KEYWORD,
DataType.TEXT,
DataType.BOOLEAN,
DataType.DATETIME,
DataType.DATE_NANOS,
DataType.DOUBLE,
DataType.INTEGER,
DataType.LONG,
DataType.UNSIGNED_LONG,
DataType.IP,
DataType.VERSION,
DataType.CARTESIAN_POINT,
DataType.GEO_POINT,
DataType.CARTESIAN_SHAPE,
DataType.GEO_SHAPE,
DataType.NULL
);

private static final List<DataType> TYPES;
static {
List<DataType> t = List.of(
DataType.KEYWORD,
DataType.TEXT,
DataType.BOOLEAN,
DataType.DATETIME,
DataType.DOUBLE,
DataType.INTEGER,
DataType.LONG,
DataType.UNSIGNED_LONG,
DataType.IP,
DataType.VERSION,
DataType.CARTESIAN_POINT,
DataType.GEO_POINT,
DataType.CARTESIAN_SHAPE,
DataType.GEO_SHAPE,
DataType.NULL
);
if (Build.current().isSnapshot()) {
t.add(DataType.DATE_NANOS);
}
TYPES = new ArrayList<>(t);
}

public CaseTests(@Name("TestCase") Supplier<TestCaseSupplier.TestCase> testCaseSupplier) {
this.testCase = testCaseSupplier.get();
Expand Down
19 changes: 12 additions & 7 deletions .../esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4431,11 +4431,16 @@ public void testReplaceSortByExpressionsWithStats() {
* \_EsRelation[test][_meta_field{f}#13, emp_no{f}#7, first_name{f}#8, ge..]
*/
public void testInlinestatsNestedExpressionsInGroups() {
var plan = optimizedPlan("""
var query = """
FROM test
| INLINESTATS c = COUNT(salary) by emp_no % 2
""");

""";
if (Build.current().isSnapshot() == false) {
var e = expectThrows(ParsingException.class, () -> analyze(query));
assertThat(e.getMessage(), containsString("line 2:3: mismatched input 'INLINESTATS' expecting {"));
return;
}
var plan = optimizedPlan(query);
var limit = as(plan, Limit.class);
var agg = as(limit.child(), InlineStats.class);
var groupings = agg.groupings();
Expand Down Expand Up @@ -4862,9 +4867,9 @@ public void testLookupSimple() {
FROM test
| RENAME languages AS int
| LOOKUP int_number_names ON int""";
if (Build.current().isProductionRelease()) {
if (Build.current().isSnapshot() == false) {
var e = expectThrows(ParsingException.class, () -> analyze(query));
assertThat(e.getMessage(), containsString("line 3:4: LOOKUP is in preview and only available in SNAPSHOT build"));
assertThat(e.getMessage(), containsString("line 3:3: mismatched input 'LOOKUP' expecting {"));
return;
}
var plan = optimizedPlan(query);
Expand Down Expand Up @@ -4941,9 +4946,9 @@ public void testLookupStats() {
| RENAME languages AS int
| LOOKUP int_number_names ON int
| STATS MIN(emp_no) BY name""";
if (Build.current().isProductionRelease()) {
if (Build.current().isSnapshot() == false) {
var e = expectThrows(ParsingException.class, () -> analyze(query));
assertThat(e.getMessage(), containsString("line 3:4: LOOKUP is in preview and only available in SNAPSHOT build"));
assertThat(e.getMessage(), containsString("line 3:3: mismatched input 'LOOKUP' expecting {"));
return;
}
var plan = optimizedPlan(query);
Expand Down
12 changes: 6 additions & 6 deletions ...esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4527,9 +4527,9 @@ public void testLookupSimple() {
FROM test
| RENAME languages AS int
| LOOKUP int_number_names ON int""";
if (Build.current().isProductionRelease()) {
if (Build.current().isSnapshot() == false) {
var e = expectThrows(ParsingException.class, () -> analyze(query));
assertThat(e.getMessage(), containsString("line 3:4: LOOKUP is in preview and only available in SNAPSHOT build"));
assertThat(e.getMessage(), containsString("line 3:3: mismatched input 'LOOKUP' expecting {"));
return;
}
PhysicalPlan plan = physicalPlan(query);
Expand Down Expand Up @@ -4576,9 +4576,9 @@ public void testLookupThenProject() {
| LOOKUP int_number_names ON int
| RENAME int AS languages, name AS lang_name
| KEEP emp_no, languages, lang_name""";
if (Build.current().isProductionRelease()) {
if (Build.current().isSnapshot() == false) {
var e = expectThrows(ParsingException.class, () -> analyze(query));
assertThat(e.getMessage(), containsString("line 5:4: LOOKUP is in preview and only available in SNAPSHOT build"));
assertThat(e.getMessage(), containsString("line 5:3: mismatched input 'LOOKUP' expecting {"));
return;
}
PhysicalPlan plan = optimizedPlan(physicalPlan(query));
Expand Down Expand Up @@ -4633,9 +4633,9 @@ public void testLookupThenTopN() {
| RENAME name AS languages
| KEEP languages, emp_no
| SORT languages ASC, emp_no ASC""";
if (Build.current().isProductionRelease()) {
if (Build.current().isSnapshot() == false) {
var e = expectThrows(ParsingException.class, () -> analyze(query));
assertThat(e.getMessage(), containsString("line 3:4: LOOKUP is in preview and only available in SNAPSHOT build"));
assertThat(e.getMessage(), containsString("line 3:3: mismatched input 'LOOKUP' expecting {"));
return;
}
var plan = physicalPlan(query);
Expand Down
38 changes: 25 additions & 13 deletions ...k/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@
//@TestLogging(value = "org.elasticsearch.xpack.esql:TRACE", reason = "debug")
public class StatementParserTests extends AbstractStatementParserTests {

private static final LogicalPlan PROCESSING_CMD_INPUT = new Row(EMPTY, List.of(new Alias(EMPTY, "a", integer(1))));

public void testRowCommand() {
assertEquals(
new Row(EMPTY, List.of(new Alias(EMPTY, "a", integer(1)), new Alias(EMPTY, "b", integer(2)))),
Expand Down Expand Up @@ -313,7 +315,12 @@ public void testAggsWithGroupKeyAsAgg() throws Exception {
}

public void testInlineStatsWithGroups() {
assumeTrue("INLINESTATS requires snapshot builds", Build.current().isSnapshot());
var query = "inlinestats b = min(a) by c, d.e";
if (Build.current().isSnapshot() == false) {
var e = expectThrows(ParsingException.class, () -> processingCommand(query));
assertThat(e.getMessage(), containsString("line 1:13: mismatched input 'inlinestats' expecting {"));
return;
}
assertEquals(
new InlineStats(
EMPTY,
Expand All @@ -325,12 +332,17 @@ public void testInlineStatsWithGroups() {
attribute("d.e")
)
),
processingCommand("inlinestats b = min(a) by c, d.e")
processingCommand(query)
);
}

public void testInlineStatsWithoutGroups() {
assumeTrue("INLINESTATS requires snapshot builds", Build.current().isSnapshot());
var query = "inlinestats min(a), c = 1";
if (Build.current().isSnapshot() == false) {
var e = expectThrows(ParsingException.class, () -> processingCommand(query));
assertThat(e.getMessage(), containsString("line 1:13: mismatched input 'inlinestats' expecting {"));
return;
}
assertEquals(
new InlineStats(
EMPTY,
Expand All @@ -341,7 +353,7 @@ public void testInlineStatsWithoutGroups() {
new Alias(EMPTY, "c", integer(1))
)
),
processingCommand("inlinestats min(a), c = 1")
processingCommand(query)
);
}

Expand Down Expand Up @@ -389,6 +401,7 @@ public void testStringAsIndexPattern() {
}

public void testStringAsLookupIndexPattern() {
assumeTrue("requires snapshot build", Build.current().isSnapshot());
assertStringAsLookupIndexPattern("foo", "ROW x = 1 | LOOKUP \"foo\" ON j");
assertStringAsLookupIndexPattern("test-*", """
ROW x = 1 | LOOKUP "test-*" ON j
Expand Down Expand Up @@ -440,6 +453,7 @@ public void testInvalidQuotingAsFromIndexPattern() {
}

public void testInvalidQuotingAsMetricsIndexPattern() {
assumeTrue("requires snapshot build", Build.current().isSnapshot());
expectError("METRICS \"foo", ": token recognition error at: '\"foo'");
expectError("METRICS \"foo | LIMIT 1", ": token recognition error at: '\"foo | LIMIT 1'");
expectError("METRICS \"\"\"foo", ": token recognition error at: '\"'");
Expand All @@ -456,6 +470,7 @@ public void testInvalidQuotingAsMetricsIndexPattern() {
}

public void testInvalidQuotingAsLookupIndexPattern() {
assumeTrue("requires snapshot builds", Build.current().isSnapshot());
expectError("ROW x = 1 | LOOKUP \"foo ON j", ": token recognition error at: '\"foo ON j'");
expectError("ROW x = 1 | LOOKUP \"\"\"foo ON j", ": token recognition error at: '\"foo ON j'");

Expand Down Expand Up @@ -1453,9 +1468,9 @@ public void testQuotedName() {
}

private void assertStringAsIndexPattern(String string, String statement) {
if (Build.current().isProductionRelease() && statement.contains("METRIC")) {
var e = expectThrows(IllegalArgumentException.class, () -> statement(statement));
assertThat(e.getMessage(), containsString("METRICS command currently requires a snapshot build"));
if (Build.current().isSnapshot() == false && statement.contains("METRIC")) {
var e = expectThrows(ParsingException.class, () -> statement(statement));
assertThat(e.getMessage(), containsString("mismatched input 'METRICS' expecting {"));
return;
}
LogicalPlan from = statement(statement);
Expand All @@ -1465,7 +1480,7 @@ private void assertStringAsIndexPattern(String string, String statement) {
}

private void assertStringAsLookupIndexPattern(String string, String statement) {
if (Build.current().isProductionRelease()) {
if (Build.current().isSnapshot() == false) {
var e = expectThrows(ParsingException.class, () -> statement(statement));
assertThat(e.getMessage(), containsString("line 1:14: LOOKUP is in preview and only available in SNAPSHOT build"));
return;
Expand Down Expand Up @@ -1533,9 +1548,9 @@ public void testInlineConvertWithNonexistentType() {

public void testLookup() {
String query = "ROW a = 1 | LOOKUP t ON j";
if (Build.current().isProductionRelease()) {
if (Build.current().isSnapshot() == false) {
var e = expectThrows(ParsingException.class, () -> statement(query));
assertThat(e.getMessage(), containsString("line 1:14: LOOKUP is in preview and only available in SNAPSHOT build"));
assertThat(e.getMessage(), containsString("line 1:13: mismatched input 'LOOKUP' expecting {"));
return;
}
var plan = statement(query);
Expand Down Expand Up @@ -1715,7 +1730,4 @@ public void testMetricWithGroupKeyAsAgg() {
expectVerificationError(query, "grouping key [a] already specified in the STATS BY clause");
}
}

private static final LogicalPlan PROCESSING_CMD_INPUT = new Row(EMPTY, List.of(new Alias(EMPTY, "a", integer(1))));

}
Loading

0 comments on commit 40db731

Please sign in to comment.