Skip to content

Commit 915bac1

Browse files
luigidellaquilaluigidellaquila
committed
Refactoring for valid fields
1 parent ec65d0b commit 915bac1

File tree

1 file changed

+77
-32
lines changed

1 file changed

+77
-32
lines changed

‎x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/EsqlQueryGenerator.java‎

Lines changed: 77 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ private static String grok(List<Column> previousOutput) {
163163
result.append(randomIdentifier());
164164
} else {
165165
String fieldName = randomRawName(previousOutput);
166-
if (fieldName.isEmpty()) { // it's a bug, managed later, skipping for now
166+
if (fieldName == null) {
167167
fieldName = randomIdentifier();
168168
}
169169
result.append(fieldName);
@@ -191,7 +191,7 @@ private static String dissect(List<Column> previousOutput) {
191191
result.append(randomIdentifier());
192192
} else {
193193
String fieldName = randomRawName(previousOutput);
194-
if (fieldName.isEmpty()) { // it's a bug, managed later, skipping for now
194+
if (fieldName == null) {
195195
fieldName = randomIdentifier();
196196
}
197197
result.append(fieldName);
@@ -210,6 +210,9 @@ private static String keep(List<Column> previousOutput) {
210210
proj.add("*");
211211
} else {
212212
String name = randomName(previousOutput);
213+
if (name == null) {
214+
continue;
215+
}
213216
if (name.length() > 1 && name.startsWith("`") == false && randomIntBetween(0, 100) < 10) {
214217
if (randomBoolean()) {
215218
name = name.substring(0, randomIntBetween(1, name.length() - 1)) + "*";
@@ -220,34 +223,42 @@ private static String keep(List<Column> previousOutput) {
220223
proj.add(name);
221224
}
222225
}
226+
if (proj.isEmpty()) {
227+
return "";
228+
}
223229
return " | keep " + proj.stream().collect(Collectors.joining(", "));
224230
}
225231

226232
private static String randomName(List<Column> previousOutput) {
227233
String result = randomRawName(previousOutput);
228-
if (result.isEmpty() // bug https://github.com/elastic/elasticsearch/issues/125870, we'll manage it as an error later
229-
|| result.contains("<")
230-
|| result.contains(">")
231-
|| (randomBoolean() && result.contains("*") == false)) {
234+
if (result == null) {
235+
return null;
236+
}
237+
if (randomBoolean() && result.contains("*") == false) {
232238
result = "`" + result + "`";
233239
}
234240
return result;
235241
}
236242

243+
/**
244+
* Returns a field name from a list of columns.
245+
* Could be null if none of the fields can be considered
246+
*/
237247
private static String randomRawName(List<Column> previousOutput) {
238-
// we need to exclude <all-fields-projected>
239-
// https://github.com/elastic/elasticsearch/issues/121741
240-
String result = randomFrom(previousOutput.stream().filter(x -> x.name().equals("<all-fields-projected>") == false).toList()).name();
248+
var list = previousOutput.stream().filter(EsqlQueryGenerator::fieldCanBeUsed).toList();
249+
if (list == null) {
250+
return null;
251+
}
252+
String result = randomFrom(list).name();
241253
return result;
242254
}
243255

256+
/**
257+
* Returns a field that can be used for grouping.
258+
* Can return null
259+
*/
244260
private static String randomGroupableName(List<Column> previousOutput) {
245-
// we need to exclude <all-fields-projected>
246-
// https://github.com/elastic/elasticsearch/issues/121741
247-
var candidates = previousOutput.stream()
248-
.filter(EsqlQueryGenerator::groupable)
249-
.filter(x -> x.name().equals("<all-fields-projected>") == false)
250-
.toList();
261+
var candidates = previousOutput.stream().filter(EsqlQueryGenerator::groupable).filter(EsqlQueryGenerator::fieldCanBeUsed).toList();
251262
if (candidates.isEmpty()) {
252263
return null;
253264
}
@@ -263,13 +274,12 @@ private static boolean groupable(Column col) {
263274
|| col.type.equals("version");
264275
}
265276

277+
/**
278+
* returns a field that can be sorted.
279+
* Null if no fields are sortable.
280+
*/
266281
private static String randomSortableName(List<Column> previousOutput) {
267-
// we need to exclude <all-fields-projected>
268-
// https://github.com/elastic/elasticsearch/issues/121741
269-
var candidates = previousOutput.stream()
270-
.filter(EsqlQueryGenerator::sortable)
271-
.filter(x -> x.name().equals("<all-fields-projected>") == false)
272-
.toList();
282+
var candidates = previousOutput.stream().filter(EsqlQueryGenerator::sortable).filter(EsqlQueryGenerator::fieldCanBeUsed).toList();
273283
if (candidates.isEmpty()) {
274284
return null;
275285
}
@@ -293,10 +303,18 @@ private static String rename(List<Column> previousOutput) {
293303
for (Column column : previousOutput) {
294304
nameToType.put(column.name, column.type);
295305
}
296-
List<String> names = new ArrayList<>(previousOutput.stream().map(Column::name).collect(Collectors.toList()));
306+
List<String> names = new ArrayList<>(
307+
previousOutput.stream().filter(EsqlQueryGenerator::fieldCanBeUsed).map(Column::name).collect(Collectors.toList())
308+
);
309+
if (names.isEmpty()) {
310+
return "";
311+
}
297312
for (int i = 0; i < n; i++) {
313+
if (names.isEmpty()) {
314+
break;
315+
}
298316
var name = randomFrom(names);
299-
if (name.equals("<all-fields-projected>") || nameToType.get(name).endsWith("_range")) {
317+
if (nameToType.get(name).endsWith("_range")) {
300318
// ranges are not fully supported yet
301319
continue;
302320
}
@@ -309,16 +327,11 @@ private static String rename(List<Column> previousOutput) {
309327
} else {
310328
newName = names.get(randomIntBetween(0, names.size() - 1));
311329
}
312-
if (newName.equals("<all-fields-projected>")) { // it's a bug, managed as an error later
313-
continue;
314-
}
315330
nameToType.put(newName, nameToType.get(name));
316-
if (name.length() == 0 // https://github.com/elastic/elasticsearch/issues/125870, we'll manage it as an error later
317-
|| (randomBoolean() && name.startsWith("`") == false)) {
331+
if (randomBoolean() && name.startsWith("`") == false) {
318332
name = "`" + name + "`";
319333
}
320-
if (newName.length() == 0 // https://github.com/elastic/elasticsearch/issues/125870, we'll manage it as an error later
321-
|| (randomBoolean() && newName.startsWith("`") == false)) {
334+
if (randomBoolean() && newName.startsWith("`") == false) {
322335
newName = "`" + newName + "`";
323336
}
324337
proj.add(name + " AS " + newName);
@@ -337,6 +350,9 @@ private static String drop(List<Column> previousOutput) {
337350
Set<String> proj = new HashSet<>();
338351
for (int i = 0; i < n; i++) {
339352
String name = randomRawName(previousOutput);
353+
if (name == null) {
354+
continue;
355+
}
340356
if (name.length() > 1 && name.startsWith("`") == false && randomIntBetween(0, 100) < 10) {
341357
if (randomBoolean()) {
342358
name = name.substring(0, randomIntBetween(1, name.length() - 1)) + "*";
@@ -348,6 +364,9 @@ private static String drop(List<Column> previousOutput) {
348364
}
349365
proj.add(name);
350366
}
367+
if (proj.isEmpty()) {
368+
return "";
369+
}
351370
return " | drop " + proj.stream().collect(Collectors.joining(", "));
352371
}
353372

@@ -368,7 +387,11 @@ private static String sort(List<Column> previousOutput) {
368387
}
369388

370389
private static String mvExpand(List<Column> previousOutput) {
371-
return " | mv_expand " + randomName(previousOutput);
390+
String toExpand = randomName(previousOutput);
391+
if (toExpand == null) {
392+
return ""; // no columns to expand
393+
}
394+
return " | mv_expand " + toExpand;
372395
}
373396

374397
private static String eval(List<Column> previousOutput) {
@@ -381,6 +404,9 @@ private static String eval(List<Column> previousOutput) {
381404
name = randomIdentifier();
382405
} else {
383406
name = randomName(previousOutput);
407+
if (name == null) {
408+
name = randomIdentifier();
409+
}
384410
}
385411
String expression = expression(previousOutput);
386412
if (i > 0) {
@@ -395,7 +421,10 @@ private static String eval(List<Column> previousOutput) {
395421
}
396422

397423
private static String stats(List<Column> previousOutput) {
398-
List<Column> nonNull = previousOutput.stream().filter(x -> x.type().equals("null") == false).collect(Collectors.toList());
424+
List<Column> nonNull = previousOutput.stream()
425+
.filter(EsqlQueryGenerator::fieldCanBeUsed)
426+
.filter(x -> x.type().equals("null") == false)
427+
.collect(Collectors.toList());
399428
if (nonNull.isEmpty()) {
400429
return ""; // cannot do any stats, just skip
401430
}
@@ -407,6 +436,9 @@ private static String stats(List<Column> previousOutput) {
407436
name = randomIdentifier();
408437
} else {
409438
name = randomName(previousOutput);
439+
if (name == null) {
440+
name = randomIdentifier();
441+
}
410442
}
411443
String expression = agg(nonNull);
412444
if (i > 0) {
@@ -438,6 +470,9 @@ private static String agg(List<Column> previousOutput) {
438470
}
439471
// all types
440472
name = randomName(previousOutput);
473+
if (name == null) {
474+
return "count(*)";
475+
}
441476
return switch (randomIntBetween(0, 2)) {
442477
case 0 -> "count(*)";
443478
case 1 -> "count(" + name + ")";
@@ -534,4 +569,14 @@ private static String randomIdentifier() {
534569
return randomAlphaOfLength(randomIntBetween(8, 12));
535570
}
536571

572+
private static boolean fieldCanBeUsed(Column field) {
573+
return (
574+
// https://github.com/elastic/elasticsearch/issues/121741
575+
field.name().equals("<all-fields-projected>")
576+
// this is a known pathological case, no need to test it for now
577+
|| field.name().equals("<no-fields>")
578+
// https://github.com/elastic/elasticsearch/issues/125870
579+
|| field.name().isEmpty()) == false;
580+
}
581+
537582
}

0 commit comments

Comments
 (0)