From fa6845b7235b288ba66ceaff57db7369acc10336 Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Wed, 30 Apr 2025 13:30:59 +0200 Subject: [PATCH 1/4] ES|QL: fix generative tests --- muted-tests.yml | 3 --- .../xpack/esql/qa/rest/generative/EsqlQueryGenerator.java | 2 ++ 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/muted-tests.yml b/muted-tests.yml index c16ddc48164c9..100c3db568cca 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -441,9 +441,6 @@ tests: - class: org.elasticsearch.action.admin.cluster.state.TransportClusterStateActionDisruptionIT method: testLocalRequestWaitsForMetadata issue: https://github.com/elastic/elasticsearch/issues/127466 -- class: org.elasticsearch.xpack.esql.qa.single_node.GenerativeIT - method: test - issue: https://github.com/elastic/elasticsearch/issues/127536 - class: org.elasticsearch.xpack.esql.qa.mixed.MixedClusterEsqlSpecIT method: test {union_types.MultiIndexSortIpStringEval ASYNC} issue: https://github.com/elastic/elasticsearch/issues/127537 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 4662867b640f1..52e969eb5229e 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 @@ -226,6 +226,8 @@ 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 + || result.contains("<") + || result.contains(">") || (randomBoolean() && result.contains("*") == false)) { result = "`" + result + "`"; } From 915bac156bf62aaf9cdaba98fc862fd95b90a3c3 Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Fri, 2 May 2025 09:57:42 +0200 Subject: [PATCH 2/4] Refactoring for valid fields --- .../rest/generative/EsqlQueryGenerator.java | 109 +++++++++++++----- 1 file changed, 77 insertions(+), 32 deletions(-) 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 52e969eb5229e..41aa89b4359a6 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 @@ -163,7 +163,7 @@ private static String grok(List previousOutput) { result.append(randomIdentifier()); } else { String fieldName = randomRawName(previousOutput); - if (fieldName.isEmpty()) { // it's a bug, managed later, skipping for now + if (fieldName == null) { fieldName = randomIdentifier(); } result.append(fieldName); @@ -191,7 +191,7 @@ private static String dissect(List previousOutput) { result.append(randomIdentifier()); } else { String fieldName = randomRawName(previousOutput); - if (fieldName.isEmpty()) { // it's a bug, managed later, skipping for now + if (fieldName == null) { fieldName = randomIdentifier(); } result.append(fieldName); @@ -210,6 +210,9 @@ private static String keep(List previousOutput) { proj.add("*"); } else { String name = randomName(previousOutput); + if (name == null) { + continue; + } if (name.length() > 1 && name.startsWith("`") == false && randomIntBetween(0, 100) < 10) { if (randomBoolean()) { name = name.substring(0, randomIntBetween(1, name.length() - 1)) + "*"; @@ -220,34 +223,42 @@ private static String keep(List previousOutput) { proj.add(name); } } + if (proj.isEmpty()) { + return ""; + } return " | keep " + proj.stream().collect(Collectors.joining(", ")); } 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 - || result.contains("<") - || result.contains(">") - || (randomBoolean() && result.contains("*") == false)) { + if (result == null) { + return null; + } + if (randomBoolean() && result.contains("*") == false) { result = "`" + result + "`"; } return result; } + /** + * Returns a field name from a list of columns. + * Could be null if none of the fields can be considered + */ private static String randomRawName(List previousOutput) { - // we need to exclude - // https://github.com/elastic/elasticsearch/issues/121741 - String result = randomFrom(previousOutput.stream().filter(x -> x.name().equals("") == false).toList()).name(); + var list = previousOutput.stream().filter(EsqlQueryGenerator::fieldCanBeUsed).toList(); + if (list == null) { + return null; + } + String result = randomFrom(list).name(); return result; } + /** + * Returns a field that can be used for grouping. + * Can return null + */ private static String randomGroupableName(List previousOutput) { - // we need to exclude - // https://github.com/elastic/elasticsearch/issues/121741 - var candidates = previousOutput.stream() - .filter(EsqlQueryGenerator::groupable) - .filter(x -> x.name().equals("") == false) - .toList(); + var candidates = previousOutput.stream().filter(EsqlQueryGenerator::groupable).filter(EsqlQueryGenerator::fieldCanBeUsed).toList(); if (candidates.isEmpty()) { return null; } @@ -263,13 +274,12 @@ private static boolean groupable(Column col) { || col.type.equals("version"); } + /** + * returns a field that can be sorted. + * Null if no fields are sortable. + */ private static String randomSortableName(List previousOutput) { - // we need to exclude - // https://github.com/elastic/elasticsearch/issues/121741 - var candidates = previousOutput.stream() - .filter(EsqlQueryGenerator::sortable) - .filter(x -> x.name().equals("") == false) - .toList(); + var candidates = previousOutput.stream().filter(EsqlQueryGenerator::sortable).filter(EsqlQueryGenerator::fieldCanBeUsed).toList(); if (candidates.isEmpty()) { return null; } @@ -293,10 +303,18 @@ private static String rename(List previousOutput) { for (Column column : previousOutput) { nameToType.put(column.name, column.type); } - List names = new ArrayList<>(previousOutput.stream().map(Column::name).collect(Collectors.toList())); + List names = new ArrayList<>( + previousOutput.stream().filter(EsqlQueryGenerator::fieldCanBeUsed).map(Column::name).collect(Collectors.toList()) + ); + if (names.isEmpty()) { + return ""; + } for (int i = 0; i < n; i++) { + if (names.isEmpty()) { + break; + } var name = randomFrom(names); - if (name.equals("") || nameToType.get(name).endsWith("_range")) { + if (nameToType.get(name).endsWith("_range")) { // ranges are not fully supported yet continue; } @@ -309,16 +327,11 @@ 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)) { + if (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)) { + if (randomBoolean() && newName.startsWith("`") == false) { newName = "`" + newName + "`"; } proj.add(name + " AS " + newName); @@ -337,6 +350,9 @@ private static String drop(List previousOutput) { Set proj = new HashSet<>(); for (int i = 0; i < n; i++) { String name = randomRawName(previousOutput); + if (name == null) { + continue; + } if (name.length() > 1 && name.startsWith("`") == false && randomIntBetween(0, 100) < 10) { if (randomBoolean()) { name = name.substring(0, randomIntBetween(1, name.length() - 1)) + "*"; @@ -348,6 +364,9 @@ private static String drop(List previousOutput) { } proj.add(name); } + if (proj.isEmpty()) { + return ""; + } return " | drop " + proj.stream().collect(Collectors.joining(", ")); } @@ -368,7 +387,11 @@ private static String sort(List previousOutput) { } private static String mvExpand(List previousOutput) { - return " | mv_expand " + randomName(previousOutput); + String toExpand = randomName(previousOutput); + if (toExpand == null) { + return ""; // no columns to expand + } + return " | mv_expand " + toExpand; } private static String eval(List previousOutput) { @@ -381,6 +404,9 @@ private static String eval(List previousOutput) { name = randomIdentifier(); } else { name = randomName(previousOutput); + if (name == null) { + name = randomIdentifier(); + } } String expression = expression(previousOutput); if (i > 0) { @@ -395,7 +421,10 @@ private static String eval(List previousOutput) { } private static String stats(List previousOutput) { - List nonNull = previousOutput.stream().filter(x -> x.type().equals("null") == false).collect(Collectors.toList()); + List nonNull = previousOutput.stream() + .filter(EsqlQueryGenerator::fieldCanBeUsed) + .filter(x -> x.type().equals("null") == false) + .collect(Collectors.toList()); if (nonNull.isEmpty()) { return ""; // cannot do any stats, just skip } @@ -407,6 +436,9 @@ private static String stats(List previousOutput) { name = randomIdentifier(); } else { name = randomName(previousOutput); + if (name == null) { + name = randomIdentifier(); + } } String expression = agg(nonNull); if (i > 0) { @@ -438,6 +470,9 @@ private static String agg(List previousOutput) { } // all types name = randomName(previousOutput); + if (name == null) { + return "count(*)"; + } return switch (randomIntBetween(0, 2)) { case 0 -> "count(*)"; case 1 -> "count(" + name + ")"; @@ -534,4 +569,14 @@ private static String randomIdentifier() { return randomAlphaOfLength(randomIntBetween(8, 12)); } + private static boolean fieldCanBeUsed(Column field) { + return ( + // https://github.com/elastic/elasticsearch/issues/121741 + field.name().equals("") + // this is a known pathological case, no need to test it for now + || field.name().equals("") + // https://github.com/elastic/elasticsearch/issues/125870 + || field.name().isEmpty()) == false; + } + } From e80b88164f2f623545dfa60c157eff908067b8fb Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Mon, 5 May 2025 13:19:00 +0200 Subject: [PATCH 3/4] Remove condition for fixed bug --- .../xpack/esql/qa/rest/generative/EsqlQueryGenerator.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) 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 41aa89b4359a6..e5c822763398d 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 @@ -574,9 +574,7 @@ private static boolean fieldCanBeUsed(Column field) { // https://github.com/elastic/elasticsearch/issues/121741 field.name().equals("") // this is a known pathological case, no need to test it for now - || field.name().equals("") - // https://github.com/elastic/elasticsearch/issues/125870 - || field.name().isEmpty()) == false; + || field.name().equals("")) == false; } } From 95b2876f4ed6be3faf3b4624915cc907b38c695c Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Mon, 5 May 2025 14:12:43 +0200 Subject: [PATCH 4/4] Fix --- .../xpack/esql/qa/rest/generative/EsqlQueryGenerator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 e5c822763398d..31fddae7c6859 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 @@ -246,7 +246,7 @@ private static String randomName(List previousOutput) { */ private static String randomRawName(List previousOutput) { var list = previousOutput.stream().filter(EsqlQueryGenerator::fieldCanBeUsed).toList(); - if (list == null) { + if (list.isEmpty()) { return null; } String result = randomFrom(list).name();