From fd2ccf7e948ff1417fe45f16546daf70c6213f32 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Tue, 25 Jun 2024 17:06:49 -0400 Subject: [PATCH 1/4] ESQL: Skip lookup csv tests on release builds 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 --- .../src/test/java/org/elasticsearch/xpack/esql/CsvTests.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java index fd161c8d63871..5cc93b9ce5555 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java @@ -240,6 +240,11 @@ public final void test() throws Throwable { testCase.requiredCapabilities, everyItem(in(EsqlCapabilities.CAPABILITIES)) ); + } else { + assumeFalse( + "lookup is not supported in non-snapshot releases", + testCase.requiredCapabilities.contains(EsqlCapabilities.Cap.LOOKUP_V3.capabilityName()) + ); } doTest(); From 310841181326660a5c4dea7929a8d17346b62e0a Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 28 Jun 2024 22:39:55 -0400 Subject: [PATCH 2/4] All snapshot only caps --- .../xpack/esql/action/EsqlCapabilities.java | 6 +++++- .../java/org/elasticsearch/xpack/esql/CsvTests.java | 12 ++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java index 1caf94dde5c30..48955a4d18c26 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java @@ -91,6 +91,8 @@ public enum Cap { */ ST_DISTANCE; + private final boolean snapshotOnly; + Cap() { snapshotOnly = false; }; @@ -103,7 +105,9 @@ public String capabilityName() { return name().toLowerCase(Locale.ROOT); } - private final boolean snapshotOnly; + public boolean snapshotOnly() { + return snapshotOnly; + } } public static final Set CAPABILITIES = capabilities(); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java index 5cc93b9ce5555..d8c7eae740c6f 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java @@ -241,10 +241,14 @@ public final void test() throws Throwable { everyItem(in(EsqlCapabilities.CAPABILITIES)) ); } else { - assumeFalse( - "lookup is not supported in non-snapshot releases", - testCase.requiredCapabilities.contains(EsqlCapabilities.Cap.LOOKUP_V3.capabilityName()) - ); + 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(); From e07d287a275b0dcc09c38f50d9c06244d8045ee8 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 1 Jul 2024 15:09:41 -0400 Subject: [PATCH 3/4] Moar test fix --- .../xpack/esql/analysis/AnalyzerTests.java | 24 ++++++---- .../optimizer/LogicalPlanOptimizerTests.java | 24 +++++++--- .../optimizer/PhysicalPlanOptimizerTests.java | 47 +++++++++++++------ .../esql/parser/StatementParserTests.java | 19 +++++++- 4 files changed, 84 insertions(+), 30 deletions(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java index 28d2046a0ea36..1f2ec0c236ecf 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java @@ -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; } @@ -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]")); } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java index 05c40ce5bd85f..6a9e7a4000734 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java @@ -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; @@ -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; @@ -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 @@ -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)); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java index 51cc2483d73b7..aa830caef5158 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java @@ -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; @@ -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; @@ -4213,10 +4215,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( @@ -4252,14 +4260,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")); @@ -4304,14 +4318,19 @@ public void testLookupThenProject() { * } */ 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); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java index bd4ae4ee53c10..6f960ee28cee2 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java @@ -53,6 +53,7 @@ import java.util.function.Function; import static org.elasticsearch.xpack.esql.EsqlTestUtils.as; +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.TRUE; import static org.elasticsearch.xpack.esql.core.expression.function.FunctionResolutionStrategy.DEFAULT; @@ -1274,6 +1275,11 @@ 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; @@ -1281,6 +1287,11 @@ private void assertStringAsIndexPattern(String string, String statement) { } 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); @@ -1343,7 +1354,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")); From cc9773416d4ec9810a37659cfa130f98c43d7848 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Tue, 2 Jul 2024 09:29:10 -0400 Subject: [PATCH 4/4] Spotless --- .../elasticsearch/xpack/esql/parser/StatementParserTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java index 6f960ee28cee2..8dcc87608c85b 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java @@ -53,7 +53,6 @@ import java.util.function.Function; import static org.elasticsearch.xpack.esql.EsqlTestUtils.as; -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.TRUE; import static org.elasticsearch.xpack.esql.core.expression.function.FunctionResolutionStrategy.DEFAULT;