Skip to content

ES|QL: enable generative tests #126097

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 2, 2025
Merged

ES|QL: enable generative tests #126097

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7eb8136
TODO
luigidellaquila Mar 31, 2025
0607919
Fix some generative tests
luigidellaquila Apr 1, 2025
31ca3da
Merge branch 'main' into esql/update_generative_tests_202503
luigidellaquila Apr 1, 2025
ffcb08b
more allowed errors
luigidellaquila Apr 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions ...-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/GenerativeIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,22 @@

import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters;

import org.apache.lucene.tests.util.LuceneTestCase.AwaitsFix;
import org.elasticsearch.test.TestClustersThreadFilter;
import org.elasticsearch.test.cluster.ElasticsearchCluster;
import org.elasticsearch.xpack.esql.qa.rest.generative.GenerativeRestTest;
import org.junit.ClassRule;

@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/121754")
/**
* This test generates random queries, runs them agains the CSV test dataset and checks that they don't throw unexpected exceptions.
*
* If muted, please:
* <ul>
* <li>see the error message reported in the failure and the corresponding query (it's in the logs right before the error)</li>
* <li>update the corresponding issue with the query (if there is no issue for that failure yet, create one)</li>
* <li>add a pattern that matches the error message to {@link GenerativeRestTest#ALLOWED_ERRORS}; also link the issue</li>
* <li>unmute (and possibly check that the test doesn't fail anymore)</li>
* </ul>
*/
@ThreadLeakFilters(filters = TestClustersThreadFilter.class)
public class GenerativeIT extends GenerativeRestTest {
@ClassRule
Expand Down
62 changes: 51 additions & 11 deletions ...ver/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/EsqlQueryGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@
import org.elasticsearch.xpack.esql.CsvTestsDataLoader;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -160,7 +162,11 @@ private static String grok(List<Column> previousOutput) {
if (randomBoolean()) {
result.append(randomAlphaOfLength(5));
} else {
result.append(randomName(previousOutput));
String fieldName = randomRawName(previousOutput);
if (fieldName.isEmpty()) { // it's a bug, managed later, skipping for now
randomAlphaOfLength(5);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this assign to a fieldName?
If so consider using randomIdentifier() instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aaahhh, automerged too quickly...
Yeah, it should assign to fieldName

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quick fix here #126107

}
result.append(fieldName);
}
result.append("}");
}
Expand All @@ -184,7 +190,11 @@ private static String dissect(List<Column> previousOutput) {
if (randomBoolean()) {
result.append(randomAlphaOfLength(5));
} else {
result.append(randomName(previousOutput));
String fieldName = randomRawName(previousOutput);
if (fieldName.isEmpty()) { // it's a bug, managed later, skipping for now
fieldName = randomAlphaOfLength(5);
}
result.append(fieldName);
}
result.append("}");
}
Expand All @@ -200,7 +210,7 @@ private static String keep(List<Column> previousOutput) {
proj.add("*");
} else {
String name = randomName(previousOutput);
if (name.length() > 1 && randomIntBetween(0, 100) < 10) {
if (name.length() > 1 && name.startsWith("`") == false && randomIntBetween(0, 100) < 10) {
if (randomBoolean()) {
name = name.substring(0, randomIntBetween(1, name.length() - 1)) + "*";
} else {
Expand All @@ -214,9 +224,19 @@ private static String keep(List<Column> previousOutput) {
}

private static String randomName(List<Column> previousOutput) {
String result = randomRawName(previousOutput);
if (result.isEmpty() // bug https://github.com/elastic/elasticsearch/issues/125870, we'll manage it as an error later
|| (randomBoolean() && result.contains("*") == false)) {
result = "`" + result + "`";
}
return result;
}

private static String randomRawName(List<Column> previousOutput) {
// we need to exclude <all-fields-projected>
// https://github.com/elastic/elasticsearch/issues/121741
return randomFrom(previousOutput.stream().filter(x -> x.name().equals("<all-fields-projected>") == false).toList()).name();
String result = randomFrom(previousOutput.stream().filter(x -> x.name().equals("<all-fields-projected>") == false).toList()).name();
return result;
}

private static String randomGroupableName(List<Column> previousOutput) {
Expand Down Expand Up @@ -266,21 +286,39 @@ private static boolean sortable(Column col) {
private static String rename(List<Column> previousOutput) {
int n = randomIntBetween(1, Math.min(3, previousOutput.size()));
List<String> proj = new ArrayList<>();

Map<String, String> nameToType = new HashMap<>();
for (Column column : previousOutput) {
nameToType.put(column.name, column.type);
}
List<String> names = new ArrayList<>(previousOutput.stream().map(Column::name).collect(Collectors.toList()));
for (int i = 0; i < n; i++) {
var colN = randomIntBetween(0, names.size() - 1);
if (previousOutput.get(colN).type().endsWith("_range")) {
var name = randomFrom(names);
if (name.equals("<all-fields-projected>") || nameToType.get(name).endsWith("_range")) {
// ranges are not fully supported yet
continue;
}
String name = names.remove(colN);
names.remove(name);

String newName;
if (names.isEmpty() || randomBoolean()) {
newName = randomAlphaOfLength(5);
names.add(newName);
} else {
newName = names.get(randomIntBetween(0, names.size() - 1));
}
names.add(newName);
if (newName.equals("<all-fields-projected>")) { // it's a bug, managed as an error later
continue;
}
nameToType.put(newName, nameToType.get(name));
if (name.length() == 0 // https://github.com/elastic/elasticsearch/issues/125870, we'll manage it as an error later
|| (randomBoolean() && name.startsWith("`") == false)) {
name = "`" + name + "`";
}
if (newName.length() == 0 // https://github.com/elastic/elasticsearch/issues/125870, we'll manage it as an error later
|| (randomBoolean() && newName.startsWith("`") == false)) {
newName = "`" + newName + "`";
}
proj.add(name + " AS " + newName);
}
if (proj.isEmpty()) {
Expand All @@ -296,15 +334,17 @@ private static String drop(List<Column> previousOutput) {
int n = randomIntBetween(1, previousOutput.size() - 1);
Set<String> proj = new HashSet<>();
for (int i = 0; i < n; i++) {
String name = randomName(previousOutput);
if (name.length() > 1 && randomIntBetween(0, 100) < 10) {
String name = randomRawName(previousOutput);
if (name.length() > 1 && name.startsWith("`") == false && randomIntBetween(0, 100) < 10) {
if (randomBoolean()) {
name = name.substring(0, randomIntBetween(1, name.length() - 1)) + "*";
} else {
name = "*" + name.substring(randomIntBetween(1, name.length() - 1));
}
} else if (name.startsWith("`") == false && (randomBoolean() || name.isEmpty())) {
name = "`" + name + "`";
}
proj.add(name.contains("*") ? name : "`" + name + "`");
proj.add(name);
}
return " | drop " + proj.stream().collect(Collectors.joining(", "));
}
Expand Down
22 changes: 16 additions & 6 deletions ...ver/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/GenerativeRestTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,23 +29,33 @@

public abstract class GenerativeRestTest extends ESRestTestCase {

public static final int ITERATIONS = 50;
public static final int MAX_DEPTH = 10;
public static final int ITERATIONS = 100;
public static final int MAX_DEPTH = 20;

public static final Set<String> ALLOWED_ERRORS = Set.of(
"Reference \\[.*\\] is ambiguous",
"Cannot use field \\[.*\\] due to ambiguities",
"cannot sort on .*",
"argument of \\[count_distinct\\(.*\\)\\] must",
"Cannot use field \\[.*\\] with unsupported type \\[.*_range\\]",
"Unbounded sort not supported yet",
"The field names are too complex to process", // field_caps problem
"must be \\[any type except counter types\\]", // TODO refine the generation of count()

// warnings
"Field '.*' shadowed by field at line .*",
"evaluation of \\[.*\\] failed, treating result as null", // TODO investigate?

// Awaiting fixes
"estimated row size \\[0\\] wasn't set", // https://github.com/elastic/elasticsearch/issues/121739
"unknown physical plan node \\[OrderExec\\]", // https://github.com/elastic/elasticsearch/issues/120817
"Unknown column \\[<all-fields-projected>\\]", // https://github.com/elastic/elasticsearch/issues/121741
//
"Unknown column \\[<all-fields-projected>\\]", // https://github.com/elastic/elasticsearch/issues/121741,
"Plan \\[ProjectExec\\[\\[<no-fields>.* optimized incorrectly due to missing references", // https://github.com/elastic/elasticsearch/issues/125866
"only supports KEYWORD or TEXT values, found expression", // https://github.com/elastic/elasticsearch/issues/126017
"token recognition error at: '``", // https://github.com/elastic/elasticsearch/issues/125870
"Unknown column \\[.*\\]", // https://github.com/elastic/elasticsearch/issues/126026
"Expected \\[.*\\] but was \\[.*\\]", // https://github.com/elastic/elasticsearch/issues/126030
"trying to encode an unsupported data type value for TopN", // still https://github.com/elastic/elasticsearch/issues/126030 probably
"Block cannot be cast to", // https://github.com/elastic/elasticsearch/issues/126036
"optimized incorrectly due to missing references", // https://github.com/elastic/elasticsearch/issues/116781
"The incoming YAML document exceeds the limit:" // still to investigate, but it seems to be specific to the test framework
);

Expand Down