Skip to content

Commit

Permalink
ES|QL: Fix error messages for non-snapshot tests (elastic#112824)
Browse files Browse the repository at this point in the history 
* Fix error messages
* Fix something unrelated to this PR
* Fix also TelemetryIT
  • Loading branch information
@astefan
astefan committed Sep 14, 2024
1 parent 0279c0a commit 3b33c35
Show file tree
Hide file tree
Showing 10 changed files with 167 additions and 82 deletions.
27 changes: 20 additions & 7 deletions ...in/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/TelemetryIT.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.action.ActionListener;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.node.DiscoveryNode;
Expand All @@ -23,6 +24,7 @@
import org.junit.Before;

import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -112,13 +114,20 @@ public static Iterable<Object[]> parameters() {
true
) },
new Object[] {
new Test("METRICS idx | LIMIT 10", Map.ofEntries(Map.entry("METRICS", 1), Map.entry("LIMIT", 1)), Map.ofEntries(), true) },
new Test(
"METRICS idx | LIMIT 10",
Build.current().isSnapshot() ? Map.ofEntries(Map.entry("METRICS", 1), Map.entry("LIMIT", 1)) : Collections.emptyMap(),
Map.ofEntries(),
Build.current().isSnapshot()
) },
new Object[] {
new Test(
"METRICS idx max(id) BY host | LIMIT 10",
Map.ofEntries(Map.entry("METRICS", 1), Map.entry("LIMIT", 1), Map.entry("FROM TS", 1)),
Map.ofEntries(Map.entry("MAX", 1)),
true
Build.current().isSnapshot()
? Map.ofEntries(Map.entry("METRICS", 1), Map.entry("LIMIT", 1), Map.entry("FROM TS", 1))
: Collections.emptyMap(),
Build.current().isSnapshot() ? Map.ofEntries(Map.entry("MAX", 1)) : Collections.emptyMap(),
Build.current().isSnapshot()
) },
new Object[] {
new Test(
Expand All @@ -127,9 +136,13 @@ public static Iterable<Object[]> parameters() {
| EVAL ip = to_ip(host), x = to_string(host), y = to_string(host)
| INLINESTATS max(id)
""",
Map.ofEntries(Map.entry("FROM", 1), Map.entry("EVAL", 1), Map.entry("INLINESTATS", 1)),
Map.ofEntries(Map.entry("MAX", 1), Map.entry("TO_IP", 1), Map.entry("TO_STRING", 2)),
true
Build.current().isSnapshot()
? Map.ofEntries(Map.entry("FROM", 1), Map.entry("EVAL", 1), Map.entry("INLINESTATS", 1))
: Collections.emptyMap(),
Build.current().isSnapshot()
? Map.ofEntries(Map.entry("MAX", 1), Map.entry("TO_IP", 1), Map.entry("TO_STRING", 2))
: Collections.emptyMap(),
Build.current().isSnapshot()
) }
);
}
Expand Down
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
47 changes: 29 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 @@ -27,31 +28,41 @@
import java.util.Locale;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static java.util.Collections.unmodifiableList;
import static org.elasticsearch.xpack.esql.EsqlTestUtils.randomLiteral;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.sameInstance;
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 = Stream.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
).collect(Collectors.toList());
if (Build.current().isSnapshot()) {
t.addAll(DataType.UNDER_CONSTRUCTION.keySet());
}
TYPES = unmodifiableList(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 @@ -4484,9 +4484,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 @@ -4533,9 +4533,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 @@ -4590,9 +4590,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
Loading

0 comments on commit 3b33c35

Please sign in to comment.