Skip to content

Commit 8a6fbaa

Browse files
authored
ESQL: Skip unsupported grapheme cluster test (#115258) (#115324)
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 8523068 commit 8a6fbaa

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.
@@ -387,45 +393,40 @@ public enum Cap {
387393
*/
388394
PER_AGG_FILTERING;
389395

390-
private final boolean snapshotOnly;
391-
private final FeatureFlag featureFlag;
396+
private final boolean enabled;
392397

393398
Cap() {
394-
this(false, null);
399+
this.enabled = true;
395400
};
396401

397-
Cap(boolean snapshotOnly) {
398-
this(snapshotOnly, null);
402+
Cap(boolean enabled) {
403+
this.enabled = enabled;
399404
};
400405

401406
Cap(FeatureFlag featureFlag) {
402-
this(false, featureFlag);
403-
}
404-
405-
Cap(boolean snapshotOnly, FeatureFlag featureFlag) {
406-
assert featureFlag == null || snapshotOnly == false;
407-
this.snapshotOnly = snapshotOnly;
408-
this.featureFlag = featureFlag;
407+
this.enabled = featureFlag.isEnabled();
409408
}
410409

411410
public boolean isEnabled() {
412-
if (featureFlag == null) {
413-
return Build.current().isSnapshot() || this.snapshotOnly == false;
414-
}
415-
return featureFlag.isEnabled();
411+
return enabled;
416412
}
417413

418414
public String capabilityName() {
419415
return name().toLowerCase(Locale.ROOT);
420416
}
421417
}
422418

423-
public static final Set<String> CAPABILITIES = capabilities();
419+
public static final Set<String> CAPABILITIES = capabilities(false);
424420

425-
private static Set<String> capabilities() {
421+
/**
422+
* Get a {@link Set} of all capabilities. If the {@code all} parameter is {@code false}
423+
* then only <strong>enabled</strong> capabilities are returned - otherwise <strong>all</strong>
424+
* known capabilities are returned.
425+
*/
426+
public static Set<String> capabilities(boolean all) {
426427
List<String> caps = new ArrayList<>();
427428
for (Cap cap : Cap.values()) {
428-
if (cap.isEnabled()) {
429+
if (all || cap.isEnabled()) {
429430
caps.add(cap.capabilityName());
430431
}
431432
}

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
@@ -45,7 +45,11 @@ public class Reverse extends UnaryScalarFunction {
4545
file = "string",
4646
tag = "reverseEmoji",
4747
description = "`REVERSE` works with unicode, too! It keeps unicode grapheme clusters together during reversal."
48-
) }
48+
) },
49+
note = """
50+
If Elasticsearch is running with a JDK version less than 20 then this will not properly reverse Grapheme Clusters.
51+
Elastic Cloud and the JDK bundled with Elasticsearch all use newer JDKs. But if you've explicitly shifted to an older jdk
52+
then you'll see things like "👍🏽😊" be reversed to "🏽👍😊" instead of the correct "😊👍🏽"."""
4953
)
5054
public Reverse(
5155
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)