Skip to content

Commit 581da4d

Browse files
authored
ESQL: Mark new signatures in MIN and MAX (#132980)
The `MIN` and `MAX` functions support unsigned longs in 9.2.0. The docs just show it's that unsigned long is supported but you can't tell for what version. This fixes that, applying the version. The version table is generated from the tests. So we have to annotate some test cases with `applies_to` and pass it through the test infrastructure.
1 parent bc7db6c commit 581da4d

File tree

17 files changed

+222
-80
lines changed

17 files changed

+222
-80
lines changed

docs/reference/query-languages/esql/_snippets/functions/types/max.md

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

docs/reference/query-languages/esql/_snippets/functions/types/max_over_time.md

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

docs/reference/query-languages/esql/_snippets/functions/types/min.md

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

docs/reference/query-languages/esql/_snippets/functions/types/min_over_time.md

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

x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/LookupJoinTypesIT.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -270,14 +270,17 @@ private static boolean existingIndex(Collection<TestConfigs> existing, DataType
270270

271271
/** This test generates documentation for the supported output types of the lookup join. */
272272
public void testOutputSupportedTypes() throws Exception {
273-
Map<List<DataType>, DataType> signatures = new LinkedHashMap<>();
273+
Map<List<DocsV3Support.Param>, DataType> signatures = new LinkedHashMap<>();
274274
for (TestConfigs configs : testConfigurations.values()) {
275275
if (configs.group.equals("unsupported") || configs.group.equals("union-types")) {
276276
continue;
277277
}
278278
for (TestConfig config : configs.configs.values()) {
279279
if (config instanceof TestConfigPasses) {
280-
signatures.put(List.of(config.mainType(), config.lookupType()), null);
280+
signatures.put(
281+
List.of(new DocsV3Support.Param(config.mainType(), List.of()), new DocsV3Support.Param(config.lookupType(), null)),
282+
null
283+
);
281284
}
282285
}
283286
}
@@ -767,7 +770,7 @@ private boolean isValidDataType(DataType dataType) {
767770
return UNDER_CONSTRUCTION.get(dataType) == null || UNDER_CONSTRUCTION.get(dataType).isEnabled();
768771
}
769772

770-
private static void saveJoinTypes(Supplier<Map<List<DataType>, DataType>> signatures) throws Exception {
773+
private static void saveJoinTypes(Supplier<Map<List<DocsV3Support.Param>, DataType>> signatures) throws Exception {
771774
if (System.getProperty("generateDocs") == null) {
772775
return;
773776
}

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -758,10 +758,10 @@ public static void testFunctionInfo() {
758758
for (int i = 0; i < args.size(); i++) {
759759
typesFromSignature.add(new HashSet<>());
760760
}
761-
for (Map.Entry<List<DataType>, DataType> entry : signatures(testClass).entrySet()) {
762-
List<DataType> types = entry.getKey();
761+
for (Map.Entry<List<DocsV3Support.Param>, DataType> entry : signatures(testClass).entrySet()) {
762+
List<DocsV3Support.Param> types = entry.getKey();
763763
for (int i = 0; i < args.size() && i < types.size(); i++) {
764-
typesFromSignature.get(i).add(types.get(i).esNameIfPossible());
764+
typesFromSignature.get(i).add(types.get(i).dataType().esNameIfPossible());
765765
}
766766
if (DataType.UNDER_CONSTRUCTION.containsKey(entry.getValue()) == false) {
767767
returnFromSignature.add(entry.getValue().esNameIfPossible());
@@ -840,14 +840,14 @@ public static void testFunctionLicenseChecks() throws Exception {
840840
// Go through all signatures and assert that the license is as expected
841841
signatures(testClass).forEach((signature, returnType) -> {
842842
try {
843-
License.OperationMode license = licenseChecker.invoke(signature);
843+
License.OperationMode license = licenseChecker.invoke(signature.stream().map(DocsV3Support.Param::dataType).toList());
844844
assertNotNull("License should not be null", license);
845845

846846
// Construct an instance of the class and then call it's licenseCheck method, and compare the results
847847
Object[] args = new Object[ctor.getParameterCount()];
848848
args[0] = Source.EMPTY;
849849
for (int i = 0; i < signature.size(); i++) {
850-
args[i + 1] = new Literal(Source.EMPTY, null, signature.get(i));
850+
args[i + 1] = new Literal(Source.EMPTY, null, signature.get(i).dataType());
851851
}
852852
Object instance = ctor.newInstance(args);
853853
// Check that object implements the LicenseAware interface
@@ -874,7 +874,7 @@ private static class TestCheckLicense {
874874

875875
private void assertLicenseCheck(
876876
LicenseAware licenseAware,
877-
List<DataType> signature,
877+
List<DocsV3Support.Param> signature,
878878
boolean allowsBasic,
879879
boolean allowsPlatinum,
880880
boolean allowsEnterprise
@@ -933,9 +933,9 @@ protected final void assertTypeResolutionFailure(Expression expression) {
933933
/**
934934
* Unique signatures in this test’s parameters.
935935
*/
936-
private static Map<List<DataType>, DataType> signatures;
936+
private static Map<List<DocsV3Support.Param>, DataType> signatures;
937937

938-
public static Map<List<DataType>, DataType> signatures(Class<?> testClass) {
938+
public static Map<List<DocsV3Support.Param>, DataType> signatures(Class<?> testClass) {
939939
if (signatures != null && classGeneratingSignatures == testClass) {
940940
return signatures;
941941
}
@@ -959,17 +959,17 @@ public static Map<List<DataType>, DataType> signatures(Class<?> testClass) {
959959
if (tc.getData().stream().anyMatch(t -> t.type() == DataType.NULL)) {
960960
continue;
961961
}
962-
List<DataType> types = tc.getData().stream().map(TestCaseSupplier.TypedData::type).toList();
963-
signatures.putIfAbsent(signatureTypes(testClass, types), tc.expectedType());
962+
List<DocsV3Support.Param> sig = tc.getData().stream().map(d -> new DocsV3Support.Param(d.type(), d.appliesTo())).toList();
963+
signatures.putIfAbsent(signatureTypes(testClass, sig), tc.expectedType());
964964
}
965965
return signatures;
966966
}
967967

968968
@SuppressWarnings("unchecked")
969-
private static List<DataType> signatureTypes(Class<?> testClass, List<DataType> types) {
969+
private static List<DocsV3Support.Param> signatureTypes(Class<?> testClass, List<DocsV3Support.Param> types) {
970970
try {
971971
Method method = testClass.getMethod("signatureTypes", List.class);
972-
return (List<DataType>) method.invoke(null, types);
972+
return (List<DocsV3Support.Param>) method.invoke(null, types);
973973
} catch (NoSuchMethodException ingored) {
974974
return types;
975975
} catch (Exception e) {
@@ -1053,9 +1053,9 @@ private static boolean isAggregation() {
10531053
/**
10541054
* Should this particular signature be hidden from the docs even though we test it?
10551055
*/
1056-
static boolean shouldHideSignature(List<DataType> argTypes, DataType returnType) {
1056+
static boolean shouldHideSignature(List<DocsV3Support.Param> argTypes, DataType returnType) {
10571057
for (DataType dt : DataType.UNDER_CONSTRUCTION.keySet()) {
1058-
if (returnType == dt || argTypes.contains(dt)) {
1058+
if (returnType == dt || argTypes.stream().anyMatch(p -> p.dataType() == dt)) {
10591059
return true;
10601060
}
10611061
}

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/DocsV3Support.java

Lines changed: 57 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,8 @@
9999
* and partially re-written to satisfy the above requirements.
100100
*/
101101
public abstract class DocsV3Support {
102+
public record Param(DataType dataType, List<FunctionAppliesTo> appliesTo) {}
103+
102104
private static final Logger logger = LogManager.getLogger(DocsV3Support.class);
103105

104106
private static final String DOCS_WARNING_JSON =
@@ -372,15 +374,15 @@ public License.OperationMode invoke(List<DataType> fieldTypes) throws Exception
372374
protected final String category;
373375
protected final String name;
374376
protected final FunctionDefinition definition;
375-
protected final Supplier<Map<List<DataType>, DataType>> signatures;
377+
protected final Supplier<Map<List<Param>, DataType>> signatures;
376378
protected final Callbacks callbacks;
377379
private final LicenseRequirementChecker licenseChecker;
378380

379381
protected DocsV3Support(
380382
String category,
381383
String name,
382384
Class<?> testClass,
383-
Supplier<Map<List<DataType>, DataType>> signatures,
385+
Supplier<Map<List<Param>, DataType>> signatures,
384386
Callbacks callbacks
385387
) {
386388
this(category, name, null, testClass, signatures, callbacks);
@@ -391,7 +393,7 @@ private DocsV3Support(
391393
String name,
392394
FunctionDefinition definition,
393395
Class<?> testClass,
394-
Supplier<Map<List<DataType>, DataType>> signatures,
396+
Supplier<Map<List<Param>, DataType>> signatures,
395397
Callbacks callbacks
396398
) {
397399
this.category = category;
@@ -571,7 +573,7 @@ private FunctionDocsSupport(String name, Class<?> testClass, Callbacks callbacks
571573
String name,
572574
Class<?> testClass,
573575
FunctionDefinition definition,
574-
Supplier<Map<List<DataType>, DataType>> signatures,
576+
Supplier<Map<List<Param>, DataType>> signatures,
575577
Callbacks callbacks
576578
) {
577579
super("functions", name, definition, testClass, signatures, callbacks);
@@ -662,10 +664,27 @@ private void renderFunctionNamedParams(EsqlFunctionRegistry.MapArgSignature mapA
662664
writeToTempSnippetsDir("functionNamedParams", rendered.toString());
663665
}
664666

665-
private String makeAppliesToText(FunctionAppliesTo[] functionAppliesTos, boolean preview) {
667+
/**
668+
* Build the {@code {applies_to}} annotation for the docs to tell users which version of
669+
* Elasticsearch first supported this function/operator/signature.
670+
* @param functionAppliesTos The version information for stateful Elasticsearch
671+
* @param preview Is this tech preview? Effectively just generates the
672+
* {@code serverless: preview} annotation if true and nothing if false.
673+
* @param oneLine Should we generate a single line variant of the {@code {applies_to}}
674+
* annotation compatible with tables (true) or the more readable
675+
* multi-line variant (false)?
676+
* @return Text of the {@code {applies_to}} annotation
677+
*/
678+
private static String makeAppliesToText(List<FunctionAppliesTo> functionAppliesTos, boolean preview, boolean oneLine) {
666679
StringBuilder appliesToText = new StringBuilder();
667-
if (functionAppliesTos.length > 0) {
668-
appliesToText.append("```{applies_to}\n");
680+
if (false == functionAppliesTos.isEmpty()) {
681+
if (oneLine) {
682+
appliesToText.append(" {applies_to}");
683+
appliesToText.append("`");
684+
} else {
685+
appliesToText.append("```");
686+
appliesToText.append("{applies_to}\n");
687+
}
669688
StringBuilder stackEntries = new StringBuilder();
670689

671690
for (FunctionAppliesTo appliesTo : functionAppliesTos) {
@@ -680,15 +699,21 @@ private String makeAppliesToText(FunctionAppliesTo[] functionAppliesTos, boolean
680699

681700
// Add the stack entries
682701
if (stackEntries.isEmpty() == false) {
683-
appliesToText.append("stack: ").append(stackEntries).append("\n");
702+
appliesToText.append("stack: ").append(stackEntries);
703+
if (false == oneLine) {
704+
appliesToText.append('\n');
705+
}
684706
}
685707

686708
// Only specify serverless if it's preview, using the preview boolean (GA is the default)
687709
if (preview) {
688-
appliesToText.append("serverless: preview\n");
710+
appliesToText.append("serverless: preview");
711+
if (false == oneLine) {
712+
appliesToText.append('\n');
713+
}
689714
}
690715

691-
appliesToText.append("```\n");
716+
appliesToText.append(oneLine ? "`" : "```\n");
692717
}
693718
return appliesToText.toString();
694719
}
@@ -711,7 +736,7 @@ private void renderFullLayout(FunctionInfo info, boolean hasExamples, boolean ha
711736
.replace("$NAME$", name)
712737
.replace("$CATEGORY$", category)
713738
.replace("$UPPER_NAME$", name.toUpperCase(Locale.ROOT))
714-
.replace("$APPLIES_TO$", makeAppliesToText(info.appliesTo(), info.preview()))
739+
.replace("$APPLIES_TO$", makeAppliesToText(Arrays.asList(info.appliesTo()), info.preview(), false))
715740
);
716741
for (String section : new String[] { "parameters", "description", "types" }) {
717742
rendered.append(addInclude(section));
@@ -755,7 +780,7 @@ public OperatorsDocsSupport(
755780
String name,
756781
Class<?> testClass,
757782
OperatorConfig op,
758-
Supplier<Map<List<DataType>, DataType>> signatures,
783+
Supplier<Map<List<Param>, DataType>> signatures,
759784
Callbacks callbacks
760785
) {
761786
super("operators", name, testClass, signatures, callbacks);
@@ -888,7 +913,9 @@ void renderDocsForOperators(
888913
if (mapParamInfo != null) {
889914
args.add(mapParam(mapParamInfo));
890915
} else {
891-
Param paramInfo = params[i].getAnnotation(Param.class);
916+
org.elasticsearch.xpack.esql.expression.function.Param paramInfo = params[i].getAnnotation(
917+
org.elasticsearch.xpack.esql.expression.function.Param.class
918+
);
892919
args.add(paramInfo != null ? param(paramInfo, false) : paramWithoutAnnotation(params[i].getName()));
893920
}
894921
}
@@ -956,7 +983,7 @@ public CommandsDocsSupport(
956983
Class<?> testClass,
957984
LogicalPlan command,
958985
List<EsqlFunctionRegistry.ArgSignature> args,
959-
Supplier<Map<List<DataType>, DataType>> signatures,
986+
Supplier<Map<List<Param>, DataType>> signatures,
960987
Callbacks callbacks
961988
) {
962989
super("commands", name, testClass, signatures, callbacks);
@@ -1016,12 +1043,12 @@ void renderTypes(String name, List<EsqlFunctionRegistry.ArgSignature> args) thro
10161043
}
10171044

10181045
Map<String, List<String>> compactedTable = new TreeMap<>();
1019-
for (Map.Entry<List<DataType>, DataType> sig : this.signatures.get().entrySet()) {
1046+
for (Map.Entry<List<DocsV3Support.Param>, DataType> sig : this.signatures.get().entrySet()) {
10201047
if (shouldHideSignature(sig.getKey(), sig.getValue())) {
10211048
continue;
10221049
}
1023-
String mainType = sig.getKey().getFirst().esNameIfPossible();
1024-
String secondaryType = sig.getKey().get(1).esNameIfPossible();
1050+
String mainType = sig.getKey().getFirst().dataType().esNameIfPossible();
1051+
String secondaryType = sig.getKey().get(1).dataType().esNameIfPossible();
10251052
List<String> secondaryTypes = compactedTable.computeIfAbsent(mainType, (k) -> new ArrayList<>());
10261053
secondaryTypes.add(secondaryType);
10271054
}
@@ -1079,7 +1106,7 @@ void renderTypes(String name, List<EsqlFunctionRegistry.ArgSignature> args) thro
10791106
}
10801107

10811108
List<String> table = new ArrayList<>();
1082-
for (Map.Entry<List<DataType>, DataType> sig : this.signatures.get().entrySet()) { // TODO flip to using sortedSignatures
1109+
for (Map.Entry<List<DocsV3Support.Param>, DataType> sig : this.signatures.get().entrySet()) { // TODO flip to using sortedSignatures
10831110
if (shouldHideSignature(sig.getKey(), sig.getValue())) {
10841111
continue;
10851112
}
@@ -1104,18 +1131,21 @@ void renderTypes(String name, List<EsqlFunctionRegistry.ArgSignature> args) thro
11041131

11051132
private static String getTypeRow(
11061133
List<EsqlFunctionRegistry.ArgSignature> args,
1107-
Map.Entry<List<DataType>, DataType> sig,
1134+
Map.Entry<List<Param>, DataType> sig,
11081135
List<String> argNames,
11091136
boolean showResultColumn
11101137
) {
11111138
StringBuilder b = new StringBuilder("| ");
11121139
for (int i = 0; i < sig.getKey().size(); i++) {
1113-
DataType argType = sig.getKey().get(i);
1140+
Param param = sig.getKey().get(i);
11141141
EsqlFunctionRegistry.ArgSignature argSignature = args.get(i);
11151142
if (argSignature.mapArg()) {
11161143
b.append("named parameters");
11171144
} else {
1118-
b.append(argType.esNameIfPossible());
1145+
b.append(param.dataType().esNameIfPossible());
1146+
if (param.appliesTo() != null) {
1147+
b.append(FunctionDocsSupport.makeAppliesToText(param.appliesTo(), false, true));
1148+
}
11191149
}
11201150
b.append(" | ");
11211151
}
@@ -1274,7 +1304,7 @@ void renderKibanaFunctionDefinition(
12741304
builder.endObject();
12751305
} else {
12761306
int minArgCount = (int) args.stream().filter(a -> false == a.optional()).count();
1277-
for (Map.Entry<List<DataType>, DataType> sig : sortedSignatures()) {
1307+
for (Map.Entry<List<DocsV3Support.Param>, DataType> sig : sortedSignatures()) {
12781308
if (variadic && sig.getKey().size() > args.size()) {
12791309
// For variadic functions we test much longer signatures, let’s just stop at the last one
12801310
continue;
@@ -1302,15 +1332,15 @@ void renderKibanaFunctionDefinition(
13021332
.collect(Collectors.joining(", "))
13031333
);
13041334
} else {
1305-
builder.field("type", sig.getKey().get(i).esNameIfPossible());
1335+
builder.field("type", sig.getKey().get(i).dataType().esNameIfPossible());
13061336
}
13071337
builder.field("optional", arg.optional());
13081338
String cleanedParamDesc = removeAppliesToBlocks(arg.description());
13091339
builder.field("description", cleanedParamDesc);
13101340
builder.endObject();
13111341
}
13121342
builder.endArray();
1313-
license = licenseChecker.invoke(sig.getKey());
1343+
license = licenseChecker.invoke(sig.getKey().stream().map(Param::dataType).toList());
13141344
if (license != null && license != License.OperationMode.BASIC) {
13151345
builder.field("license", license.toString());
13161346
}
@@ -1358,8 +1388,8 @@ private static String removeAppliesToBlocks(String content) {
13581388
return content.replaceAll("\\s*\\{applies_to\\}`[^`]*`\\s*", "");
13591389
}
13601390

1361-
private List<Map.Entry<List<DataType>, DataType>> sortedSignatures() {
1362-
List<Map.Entry<List<DataType>, DataType>> sortedSignatures = new ArrayList<>(signatures.get().entrySet());
1391+
private List<Map.Entry<List<DocsV3Support.Param>, DataType>> sortedSignatures() {
1392+
List<Map.Entry<List<DocsV3Support.Param>, DataType>> sortedSignatures = new ArrayList<>(signatures.get().entrySet());
13631393
sortedSignatures.sort((lhs, rhs) -> {
13641394
int maxlen = Math.max(lhs.getKey().size(), rhs.getKey().size());
13651395
for (int i = 0; i < maxlen; i++) {
@@ -1369,7 +1399,7 @@ private List<Map.Entry<List<DataType>, DataType>> sortedSignatures() {
13691399
if (rhs.getKey().size() <= i) {
13701400
return 1;
13711401
}
1372-
int c = lhs.getKey().get(i).esNameIfPossible().compareTo(rhs.getKey().get(i).esNameIfPossible());
1402+
int c = lhs.getKey().get(i).dataType().esNameIfPossible().compareTo(rhs.getKey().get(i).dataType().esNameIfPossible());
13731403
if (c != 0) {
13741404
return c;
13751405
}

0 commit comments

Comments
 (0)