From 7eb81366f947829d32dd44761fbbe0acfb8d07a7 Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Mon, 31 Mar 2025 15:24:36 +0200 Subject: [PATCH 1/3] TODO --- .../xpack/esql/qa/single_node/GenerativeIT.java | 2 +- .../esql/qa/rest/generative/EsqlQueryGenerator.java | 2 +- .../esql/qa/rest/generative/GenerativeRestTest.java | 11 ++++++----- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/GenerativeIT.java b/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/GenerativeIT.java index d322263ce9182..ff2766eb78436 100644 --- a/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/GenerativeIT.java +++ b/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/GenerativeIT.java @@ -15,7 +15,7 @@ import org.elasticsearch.xpack.esql.qa.rest.generative.GenerativeRestTest; import org.junit.ClassRule; -@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/121754") +//@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/121754") @ThreadLeakFilters(filters = TestClustersThreadFilter.class) public class GenerativeIT extends GenerativeRestTest { @ClassRule diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/EsqlQueryGenerator.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/EsqlQueryGenerator.java index 5bf13d2d9c762..cfe96b7fd199a 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/EsqlQueryGenerator.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/EsqlQueryGenerator.java @@ -277,10 +277,10 @@ private static String rename(List previousOutput) { String newName; if (names.isEmpty() || randomBoolean()) { newName = randomAlphaOfLength(5); + names.add(newName); } else { newName = names.get(randomIntBetween(0, names.size() - 1)); } - names.add(newName); proj.add(name + " AS " + newName); } if (proj.isEmpty()) { diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/GenerativeRestTest.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/GenerativeRestTest.java index 0ceeb132f5b5c..e95036a044c74 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/GenerativeRestTest.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/GenerativeRestTest.java @@ -29,8 +29,8 @@ public abstract class GenerativeRestTest extends ESRestTestCase { - public static final int ITERATIONS = 50; - public static final int MAX_DEPTH = 10; + public static final int ITERATIONS = 500; + public static final int MAX_DEPTH = 20; public static final Set ALLOWED_ERRORS = Set.of( "Reference \\[.*\\] is ambiguous", @@ -38,13 +38,14 @@ public abstract class GenerativeRestTest extends ESRestTestCase { "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 // 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 \\[\\]", // https://github.com/elastic/elasticsearch/issues/121741 + "Unknown column \\[\\]", // https://github.com/elastic/elasticsearch/issues/121741, + "Plan \\[ProjectExec\\[\\[.* optimized incorrectly due to missing references", // https://github.com/elastic/elasticsearch/issues/125866 // "The incoming YAML document exceeds the limit:" // still to investigate, but it seems to be specific to the test framework ); From 06079198158c8cc53f618c2426b340bc31fa72e1 Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Tue, 1 Apr 2025 13:09:18 +0200 Subject: [PATCH 2/3] Fix some generative tests --- .../esql/qa/single_node/GenerativeIT.java | 1 - .../rest/generative/EsqlQueryGenerator.java | 45 ++++++++++++++----- .../rest/generative/GenerativeRestTest.java | 8 +++- 3 files changed, 42 insertions(+), 12 deletions(-) diff --git a/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/GenerativeIT.java b/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/GenerativeIT.java index ff2766eb78436..89f82960013f2 100644 --- a/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/GenerativeIT.java +++ b/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/GenerativeIT.java @@ -9,7 +9,6 @@ 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; diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/EsqlQueryGenerator.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/EsqlQueryGenerator.java index cfe96b7fd199a..9373559194bb4 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/EsqlQueryGenerator.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/EsqlQueryGenerator.java @@ -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; @@ -160,7 +162,7 @@ private static String grok(List previousOutput) { if (randomBoolean()) { result.append(randomAlphaOfLength(5)); } else { - result.append(randomName(previousOutput)); + result.append(randomRawName(previousOutput)); } result.append("}"); } @@ -184,7 +186,7 @@ private static String dissect(List previousOutput) { if (randomBoolean()) { result.append(randomAlphaOfLength(5)); } else { - result.append(randomName(previousOutput)); + result.append(randomRawName(previousOutput)); } result.append("}"); } @@ -200,7 +202,7 @@ private static String keep(List 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 { @@ -214,9 +216,19 @@ private static String keep(List previousOutput) { } private static String randomName(List 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 previousOutput) { // we need to exclude // https://github.com/elastic/elasticsearch/issues/121741 - return randomFrom(previousOutput.stream().filter(x -> x.name().equals("") == false).toList()).name(); + String result = randomFrom(previousOutput.stream().filter(x -> x.name().equals("") == false).toList()).name(); + return result; } private static String randomGroupableName(List previousOutput) { @@ -266,14 +278,20 @@ private static boolean sortable(Column col) { private static String rename(List previousOutput) { int n = randomIntBetween(1, Math.min(3, previousOutput.size())); List proj = new ArrayList<>(); + + Map nameToType = new HashMap<>(); + for (Column column : previousOutput) { + nameToType.put(column.name, column.type); + } List 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 (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); @@ -281,6 +299,11 @@ private static String rename(List previousOutput) { } else { newName = names.get(randomIntBetween(0, names.size() - 1)); } + 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 + "`"; + } proj.add(name + " AS " + newName); } if (proj.isEmpty()) { @@ -296,15 +319,17 @@ private static String drop(List previousOutput) { int n = randomIntBetween(1, previousOutput.size() - 1); Set 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(", ")); } diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/GenerativeRestTest.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/GenerativeRestTest.java index e95036a044c74..cc070cf7f25cd 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/GenerativeRestTest.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/GenerativeRestTest.java @@ -40,13 +40,18 @@ public abstract class GenerativeRestTest extends ESRestTestCase { "Cannot use field \\[.*\\] with unsupported type \\[.*_range\\]", "Unbounded sort not supported yet", "The field names are too complex to process", // field_caps problem + "argument of \\[count.*\\] 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 "Unknown column \\[\\]", // https://github.com/elastic/elasticsearch/issues/121741, "Plan \\[ProjectExec\\[\\[.* 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 "The incoming YAML document exceeds the limit:" // still to investigate, but it seems to be specific to the test framework ); @@ -136,6 +141,7 @@ private List availableIndices() { CSV_DATASET_MAP.entrySet() .stream() .filter(x -> x.getValue().requiresInferenceEndpoint() == false) + .filter(x -> "partial_mapping_excluded_source_sample_data".equals(x.getKey()) == false) // TODO double-check .map(Map.Entry::getKey) .toList() ); From ffcb08b25faddfe1c02a601d5a248a892f0e21c2 Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Wed, 2 Apr 2025 09:44:33 +0200 Subject: [PATCH 3/3] more allowed errors --- .../esql/qa/single_node/GenerativeIT.java | 12 ++++++++++- .../rest/generative/EsqlQueryGenerator.java | 21 ++++++++++++++++--- .../rest/generative/GenerativeRestTest.java | 9 +++++--- 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/GenerativeIT.java b/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/GenerativeIT.java index 89f82960013f2..19f96d89690a8 100644 --- a/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/GenerativeIT.java +++ b/x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/GenerativeIT.java @@ -14,7 +14,17 @@ 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: + *
    + *
  • see the error message reported in the failure and the corresponding query (it's in the logs right before the error)
    • + *
  • update the corresponding issue with the query (if there is no issue for that failure yet, create one)
    • + *
  • add a pattern that matches the error message to {@link GenerativeRestTest#ALLOWED_ERRORS}; also link the issue
    • + *
  • unmute (and possibly check that the test doesn't fail anymore)
    • + *
+ */ @ThreadLeakFilters(filters = TestClustersThreadFilter.class) public class GenerativeIT extends GenerativeRestTest { @ClassRule diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/EsqlQueryGenerator.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/EsqlQueryGenerator.java index 9373559194bb4..25ab6ddd43fc6 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/EsqlQueryGenerator.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/EsqlQueryGenerator.java @@ -162,7 +162,11 @@ private static String grok(List previousOutput) { if (randomBoolean()) { result.append(randomAlphaOfLength(5)); } else { - result.append(randomRawName(previousOutput)); + String fieldName = randomRawName(previousOutput); + if (fieldName.isEmpty()) { // it's a bug, managed later, skipping for now + randomAlphaOfLength(5); + } + result.append(fieldName); } result.append("}"); } @@ -186,7 +190,11 @@ private static String dissect(List previousOutput) { if (randomBoolean()) { result.append(randomAlphaOfLength(5)); } else { - result.append(randomRawName(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("}"); } @@ -286,7 +294,7 @@ private static String rename(List previousOutput) { List names = new ArrayList<>(previousOutput.stream().map(Column::name).collect(Collectors.toList())); for (int i = 0; i < n; i++) { var name = randomFrom(names); - if (nameToType.get(name).endsWith("_range")) { + if (name.equals("") || nameToType.get(name).endsWith("_range")) { // ranges are not fully supported yet continue; } @@ -299,11 +307,18 @@ private static String rename(List previousOutput) { } else { newName = names.get(randomIntBetween(0, names.size() - 1)); } + if (newName.equals("")) { // 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()) { diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/GenerativeRestTest.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/GenerativeRestTest.java index cc070cf7f25cd..4ca00aead6f11 100644 --- a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/GenerativeRestTest.java +++ b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/GenerativeRestTest.java @@ -29,7 +29,7 @@ public abstract class GenerativeRestTest extends ESRestTestCase { - public static final int ITERATIONS = 500; + public static final int ITERATIONS = 100; public static final int MAX_DEPTH = 20; public static final Set ALLOWED_ERRORS = Set.of( @@ -40,7 +40,7 @@ public abstract class GenerativeRestTest extends ESRestTestCase { "Cannot use field \\[.*\\] with unsupported type \\[.*_range\\]", "Unbounded sort not supported yet", "The field names are too complex to process", // field_caps problem - "argument of \\[count.*\\] must be \\[any type except counter types\\]", // TODO refine the generation of count() + "must be \\[any type except counter types\\]", // TODO refine the generation of count() // warnings "Field '.*' shadowed by field at line .*", @@ -52,6 +52,10 @@ public abstract class GenerativeRestTest extends ESRestTestCase { "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 ); @@ -141,7 +145,6 @@ private List availableIndices() { CSV_DATASET_MAP.entrySet() .stream() .filter(x -> x.getValue().requiresInferenceEndpoint() == false) - .filter(x -> "partial_mapping_excluded_source_sample_data".equals(x.getKey()) == false) // TODO double-check .map(Map.Entry::getKey) .toList() );