Skip to content

Commit f38f230

Browse files
authored
ESQL: Skip unsupported grapheme cluster test (#115258)
This skips the test for reversing grapheme clusters if the node doesn't support reversing grapheme clusters. Nodes that are using a jdk before 20 won't support reversing grapheme clusters because they don't have https://bugs.openjdk.org/browse/JDK-8292387 This reworks `EsqlCapabilities` so we can easilly register it only if we're on jdk 20: ``` FN_REVERSE_GRAPHEME_CLUSTERS(Runtime.version().feature() < 20), ``` Closes #114537 Closes #114535 Closes #114536 Closes #114558 Closes #114559 Closes #114560
1 parent d4cb781 commit f38f230

File tree

7 files changed

+39
-25
lines changed

7 files changed

+39
-25
lines changed

docs/reference/esql/functions/description/reverse.asciidoc

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/reference/esql/functions/kibana/definition/reverse.json

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/reference/esql/functions/kibana/docs/reverse.md

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1236,6 +1236,7 @@ off_on_holiday:keyword | back_home_again:keyword
12361236

12371237
reverseGraphemeClusters
12381238
required_capability: fn_reverse
1239+
required_capability: fn_reverse_grapheme_clusters
12391240
ROW message = "áéíóúàèìòùâêîôû😊👍🏽🎉💖कंठाी" | EVAL message_reversed = REVERSE(message);
12401241

12411242
message:keyword | message_reversed:keyword

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ public enum Cap {
3232
*/
3333
FN_REVERSE,
3434

35+
/**
36+
* Support for reversing whole grapheme clusters. This is not supported
37+
* on JDK versions less than 20.
38+
*/
39+
FN_REVERSE_GRAPHEME_CLUSTERS(Runtime.version().feature() < 20),
40+
3541
/**
3642
* Support for function {@code CBRT}. Done in #108574.
3743
*/
@@ -133,7 +139,7 @@ public enum Cap {
133139
* - fixed variable shadowing
134140
* - fixed Join.references(), requiring breaking change to Join serialization
135141
*/
136-
LOOKUP_V4(true),
142+
LOOKUP_V4(Build.current().isSnapshot()),
137143

138144
/**
139145
* Support for requesting the "REPEAT" command.
@@ -279,7 +285,7 @@ public enum Cap {
279285
/**
280286
* Support for match operator
281287
*/
282-
MATCH_OPERATOR(true),
288+
MATCH_OPERATOR(Build.current().isSnapshot()),
283289

284290
/**
285291
* Removing support for the {@code META} keyword.
@@ -349,7 +355,7 @@ public enum Cap {
349355
/**
350356
* Supported the text categorization function "CATEGORIZE".
351357
*/
352-
CATEGORIZE(true),
358+
CATEGORIZE(Build.current().isSnapshot()),
353359

354360
/**
355361
* QSTR function
@@ -375,7 +381,7 @@ public enum Cap {
375381
/**
376382
* Support named parameters for field names.
377383
*/
378-
NAMED_PARAMETER_FOR_FIELD_AND_FUNCTION_NAMES(true),
384+
NAMED_PARAMETER_FOR_FIELD_AND_FUNCTION_NAMES(Build.current().isSnapshot()),
379385

380386
/**
381387
* Fix sorting not allowed on _source and counters.
@@ -397,45 +403,40 @@ public enum Cap {
397403
*/
398404
FUNCTION_STATS;
399405

400-
private final boolean snapshotOnly;
401-
private final FeatureFlag featureFlag;
406+
private final boolean enabled;
402407

403408
Cap() {
404-
this(false, null);
409+
this.enabled = true;
405410
};
406411

407-
Cap(boolean snapshotOnly) {
408-
this(snapshotOnly, null);
412+
Cap(boolean enabled) {
413+
this.enabled = enabled;
409414
};
410415

411416
Cap(FeatureFlag featureFlag) {
412-
this(false, featureFlag);
413-
}
414-
415-
Cap(boolean snapshotOnly, FeatureFlag featureFlag) {
416-
assert featureFlag == null || snapshotOnly == false;
417-
this.snapshotOnly = snapshotOnly;
418-
this.featureFlag = featureFlag;
417+
this.enabled = featureFlag.isEnabled();
419418
}
420419

421420
public boolean isEnabled() {
422-
if (featureFlag == null) {
423-
return Build.current().isSnapshot() || this.snapshotOnly == false;
424-
}
425-
return featureFlag.isEnabled();
421+
return enabled;
426422
}
427423

428424
public String capabilityName() {
429425
return name().toLowerCase(Locale.ROOT);
430426
}
431427
}
432428

433-
public static final Set<String> CAPABILITIES = capabilities();
429+
public static final Set<String> CAPABILITIES = capabilities(false);
434430

435-
private static Set<String> capabilities() {
431+
/**
432+
* Get a {@link Set} of all capabilities. If the {@code all} parameter is {@code false}
433+
* then only <strong>enabled</strong> capabilities are returned - otherwise <strong>all</strong>
434+
* known capabilities are returned.
435+
*/
436+
public static Set<String> capabilities(boolean all) {
436437
List<String> caps = new ArrayList<>();
437438
for (Cap cap : Cap.values()) {
438-
if (cap.isEnabled()) {
439+
if (all || cap.isEnabled()) {
439440
caps.add(cap.capabilityName());
440441
}
441442
}

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Reverse.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,11 @@ public class Reverse extends UnaryScalarFunction {
4646
file = "string",
4747
tag = "reverseEmoji",
4848
description = "`REVERSE` works with unicode, too! It keeps unicode grapheme clusters together during reversal."
49-
) }
49+
) },
50+
note = """
51+
If Elasticsearch is running with a JDK version less than 20 then this will not properly reverse Grapheme Clusters.
52+
Elastic Cloud and the JDK bundled with Elasticsearch all use newer JDKs. But if you've explicitly shifted to an older jdk
53+
then you'll see things like "👍🏽😊" be reversed to "🏽👍😊" instead of the correct "😊👍🏽"."""
5054
)
5155
public Reverse(
5256
Source source,

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ public final void test() throws Throwable {
257257
assertThat(
258258
"Capability is not included in the enabled list capabilities on a snapshot build. Spelling mistake?",
259259
testCase.requiredCapabilities,
260-
everyItem(in(EsqlCapabilities.CAPABILITIES))
260+
everyItem(in(EsqlCapabilities.capabilities(true)))
261261
);
262262
} else {
263263
for (EsqlCapabilities.Cap c : EsqlCapabilities.Cap.values()) {

0 commit comments

Comments
 (0)