Skip to content

Commit

Permalink
ESQL: Skip lookup csv tests on release builds (#110166)
Browse files Browse the repository at this point in the history 
LOOKUP isn't supported in release builds yet and it'll fail with a
helpful error message if you try it there. But some of the csv-spec
tests didn't realize that. Lots did, but these particular ones didn't.

Close #109170
  • Loading branch information
@nik9000
nik9000 committed Jul 2, 2024
1 parent 8f8d19c commit 0680d7e
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 31 deletions.
6 changes: 5 additions & 1 deletion x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ public enum Cap {
*/
DOUBLE_QUOTES_SOURCE_ENCLOSING;

private final boolean snapshotOnly;

Cap() {
snapshotOnly = false;
};
Expand All @@ -115,7 +117,9 @@ public String capabilityName() {
return name().toLowerCase(Locale.ROOT);
}

private final boolean snapshotOnly;
public boolean snapshotOnly() {
return snapshotOnly;
}
}

public static final Set<String> CAPABILITIES = capabilities();
Expand Down
9 changes: 9 additions & 0 deletions x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,15 @@ public final void test() throws Throwable {
testCase.requiredCapabilities,
everyItem(in(EsqlCapabilities.CAPABILITIES))
);
} else {
for (EsqlCapabilities.Cap c : EsqlCapabilities.Cap.values()) {
if (c.snapshotOnly()) {
assumeFalse(
c.capabilityName() + " is not supported in non-snapshot releases",
testCase.requiredCapabilities.contains(c.capabilityName())
);
}
}
}

doTest();
Expand Down
24 changes: 15 additions & 9 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 @@ -1931,7 +1931,7 @@ public void testLookup() {
| LOOKUP int_number_names ON int
""";
if (Build.current().isProductionRelease()) {
var e = expectThrows(VerificationException.class, () -> analyze(query));
var e = expectThrows(ParsingException.class, () -> analyze(query));
assertThat(e.getMessage(), containsString("line 3:4: LOOKUP is in preview and only available in SNAPSHOT build"));
return;
}
Expand Down Expand Up @@ -1982,39 +1982,45 @@ public void testLookup() {
}

public void testLookupMissingField() {
var e = expectThrows(VerificationException.class, () -> analyze("""
String query = """
FROM test
| LOOKUP int_number_names ON garbage
"""));
""";
if (Build.current().isProductionRelease()) {
assertThat(e.getMessage(), containsString("line 3:4: LOOKUP is in preview and only available in SNAPSHOT build"));
var e = expectThrows(ParsingException.class, () -> analyze(query));
assertThat(e.getMessage(), containsString("line 2:4: LOOKUP is in preview and only available in SNAPSHOT build"));
return;
}
var e = expectThrows(VerificationException.class, () -> analyze(query));
assertThat(e.getMessage(), containsString("Unknown column in lookup target [garbage]"));
}

public void testLookupMissingTable() {
var e = expectThrows(VerificationException.class, () -> analyze("""
String query = """
FROM test
| LOOKUP garbage ON a
"""));
""";
if (Build.current().isProductionRelease()) {
assertThat(e.getMessage(), containsString("line 3:4: LOOKUP is in preview and only available in SNAPSHOT build"));
var e = expectThrows(ParsingException.class, () -> analyze(query));
assertThat(e.getMessage(), containsString("line 2:4: LOOKUP is in preview and only available in SNAPSHOT build"));
return;
}
var e = expectThrows(VerificationException.class, () -> analyze(query));
assertThat(e.getMessage(), containsString("Unknown table [garbage]"));
}

public void testLookupMatchTypeWrong() {
var e = expectThrows(VerificationException.class, () -> analyze("""
String query = """
FROM test
| RENAME last_name AS int
| LOOKUP int_number_names ON int
"""));
""";
if (Build.current().isProductionRelease()) {
var e = expectThrows(ParsingException.class, () -> analyze(query));
assertThat(e.getMessage(), containsString("line 3:4: LOOKUP is in preview and only available in SNAPSHOT build"));
return;
}
var e = expectThrows(VerificationException.class, () -> analyze(query));
assertThat(e.getMessage(), containsString("column type mismatch, table column was [integer] and original column was [keyword]"));
}

Expand Down
24 changes: 18 additions & 6 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 @@ -116,6 +116,7 @@
import org.elasticsearch.xpack.esql.optimizer.rules.PushDownAndCombineLimits;
import org.elasticsearch.xpack.esql.optimizer.rules.SplitInWithFoldableValue;
import org.elasticsearch.xpack.esql.parser.EsqlParser;
import org.elasticsearch.xpack.esql.parser.ParsingException;
import org.elasticsearch.xpack.esql.plan.logical.Aggregate;
import org.elasticsearch.xpack.esql.plan.logical.Dissect;
import org.elasticsearch.xpack.esql.plan.logical.Enrich;
Expand Down Expand Up @@ -158,6 +159,7 @@
import static org.elasticsearch.xpack.esql.EsqlTestUtils.localSource;
import static org.elasticsearch.xpack.esql.EsqlTestUtils.withDefaultLimitWarning;
import static org.elasticsearch.xpack.esql.analysis.Analyzer.NO_FIELDS;
import static org.elasticsearch.xpack.esql.analysis.AnalyzerTestUtils.analyze;
import static org.elasticsearch.xpack.esql.core.expression.Literal.FALSE;
import static org.elasticsearch.xpack.esql.core.expression.Literal.NULL;
import static org.elasticsearch.xpack.esql.core.expression.Literal.TRUE;
Expand Down Expand Up @@ -5009,11 +5011,16 @@ public void testIsNullDisjunction() throws Exception {
* }
*/
public void testLookupSimple() {
var plan = optimizedPlan("""
String query = """
FROM test
| RENAME languages AS int
| LOOKUP int_number_names ON int
""");
| LOOKUP int_number_names ON int""";
if (Build.current().isProductionRelease()) {
var e = expectThrows(ParsingException.class, () -> analyze(query));
assertThat(e.getMessage(), containsString("line 3:4: LOOKUP is in preview and only available in SNAPSHOT build"));
return;
}
var plan = optimizedPlan(query);
var join = as(plan, Join.class);

// Right is the lookup table
Expand Down Expand Up @@ -5082,12 +5089,17 @@ public void testLookupSimple() {
* }
*/
public void testLookupStats() {
var plan = optimizedPlan("""
String query = """
FROM test
| RENAME languages AS int
| LOOKUP int_number_names ON int
| STATS MIN(emp_no) BY name
""");
| STATS MIN(emp_no) BY name""";
if (Build.current().isProductionRelease()) {
var e = expectThrows(ParsingException.class, () -> analyze(query));
assertThat(e.getMessage(), containsString("line 3:4: LOOKUP is in preview and only available in SNAPSHOT build"));
return;
}
var plan = optimizedPlan(query);
var limit = as(plan, Limit.class);
assertThat(limit.limit().fold(), equalTo(1000));

Expand Down
47 changes: 33 additions & 14 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 @@ -9,6 +9,7 @@

import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;

import org.elasticsearch.Build;
import org.elasticsearch.common.geo.ShapeRelation;
import org.elasticsearch.common.lucene.BytesRefs;
import org.elasticsearch.common.settings.Settings;
Expand Down Expand Up @@ -135,6 +136,7 @@
import static org.elasticsearch.xpack.esql.EsqlTestUtils.statsForMissingField;
import static org.elasticsearch.xpack.esql.EsqlTestUtils.withDefaultLimitWarning;
import static org.elasticsearch.xpack.esql.SerializationTestUtils.assertSerialization;
import static org.elasticsearch.xpack.esql.analysis.AnalyzerTestUtils.analyze;
import static org.elasticsearch.xpack.esql.core.expression.Expressions.name;
import static org.elasticsearch.xpack.esql.core.expression.Expressions.names;
import static org.elasticsearch.xpack.esql.core.expression.Order.OrderDirection.ASC;
Expand Down Expand Up @@ -4231,10 +4233,16 @@ public void testMaxQueryDepthPlusExpressionDepth() {
}

public void testLookupSimple() {
PhysicalPlan plan = physicalPlan("""
FROM test |
RENAME languages AS int |
LOOKUP int_number_names ON int""");
String query = """
FROM test
| RENAME languages AS int
| LOOKUP int_number_names ON int""";
if (Build.current().isProductionRelease()) {
var e = expectThrows(ParsingException.class, () -> analyze(query));
assertThat(e.getMessage(), containsString("line 3:4: LOOKUP is in preview and only available in SNAPSHOT build"));
return;
}
PhysicalPlan plan = physicalPlan(query);
var join = as(plan, HashJoinExec.class);
assertMap(join.matchFields().stream().map(Object::toString).toList(), matchesList().item(startsWith("int{r}")));
assertMap(
Expand Down Expand Up @@ -4270,14 +4278,20 @@ public void testLookupSimple() {
* }
*/
public void testLookupThenProject() {
PhysicalPlan plan = optimizedPlan(physicalPlan("""
String query = """
FROM employees
| SORT emp_no
| LIMIT 4
| RENAME languages AS int
| LOOKUP int_number_names ON int
| RENAME int AS languages, name AS lang_name
| KEEP emp_no, languages, lang_name"""));
| KEEP emp_no, languages, lang_name""";
if (Build.current().isProductionRelease()) {
var e = expectThrows(ParsingException.class, () -> analyze(query));
assertThat(e.getMessage(), containsString("line 5:4: LOOKUP is in preview and only available in SNAPSHOT build"));
return;
}
PhysicalPlan plan = optimizedPlan(physicalPlan(query));

var outerProject = as(plan, ProjectExec.class);
assertThat(outerProject.projections().toString(), containsString("AS lang_name"));
Expand Down Expand Up @@ -4322,14 +4336,19 @@ public void testLookupThenProject() {
* }</pre>
*/
public void testLookupThenTopN() {
var plan = physicalPlan("""
FROM employees
| RENAME languages AS int
| LOOKUP int_number_names ON int
| RENAME name AS languages
| KEEP languages, emp_no
| SORT languages ASC, emp_no ASC
""");
String query = """
FROM employees
| RENAME languages AS int
| LOOKUP int_number_names ON int
| RENAME name AS languages
| KEEP languages, emp_no
| SORT languages ASC, emp_no ASC""";
if (Build.current().isProductionRelease()) {
var e = expectThrows(ParsingException.class, () -> analyze(query));
assertThat(e.getMessage(), containsString("line 3:4: LOOKUP is in preview and only available in SNAPSHOT build"));
return;
}
var plan = physicalPlan(query);

ProjectExec outerProject = as(plan, ProjectExec.class);
TopNExec outerTopN = as(outerProject.child(), TopNExec.class);
Expand Down
18 changes: 17 additions & 1 deletion ...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 @@ -1274,13 +1274,23 @@ 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"));
return;
}
LogicalPlan from = statement(statement);
assertThat(from, instanceOf(EsqlUnresolvedRelation.class));
EsqlUnresolvedRelation table = (EsqlUnresolvedRelation) from;
assertThat(table.table().index(), is(string));
}

private void assertStringAsLookupIndexPattern(String string, String statement) {
if (Build.current().isProductionRelease()) {
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;
}
var plan = statement(statement);
var lookup = as(plan, Lookup.class);
var tableName = as(lookup.tableName(), Literal.class);
Expand Down Expand Up @@ -1343,7 +1353,13 @@ public void testInlineConvertWithNonexistentType() {
}

public void testLookup() {
var plan = statement("ROW a = 1 | LOOKUP t ON j");
String query = "ROW a = 1 | LOOKUP t ON j";
if (Build.current().isProductionRelease()) {
var e = expectThrows(ParsingException.class, () -> statement(query));
assertThat(e.getMessage(), containsString("line 1:14: LOOKUP is in preview and only available in SNAPSHOT build"));
return;
}
var plan = statement(query);
var lookup = as(plan, Lookup.class);
var tableName = as(lookup.tableName(), Literal.class);
assertThat(tableName.fold(), equalTo("t"));
Expand Down

0 comments on commit 0680d7e

Please sign in to comment.