Browse Source

Fix automatic generation of spatial function types files (#105766)

* Fix automatic generation of spatial function types files

The automatic mapping of spatial function names from class names was not working for spatial types, so the automatic generation of these files did not happen, and in fact existing files were deleted.

In addition, the generation of aggregation functions types does not yet exist at all, so the st_centroid.asciidoc file was always deleted. Until such support exists, this files contents will be moved back into the function definition file.

The railroad diagrams for syntax are now also created, however, not all functions in the documentation actually use these, and certainly none of the `TO_*` type-casting functions do, so we'll not include links to them from the docs, and leave that to the docs team to decide. Personally, while these diagrams are pretty, they contain no additional informational content, and in fact give a cluttered impression to the documentation visual appeal.

* Refined to use an annotation which is more generic
Craig Taverner 1 year ago
parent
commit
85cd204317

+ 1 - 0
docs/reference/esql/functions/signature/to_cartesianpoint.svg

@@ -0,0 +1 @@
+<svg version="1.1" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns="http://www.w3.org/2000/svg" width="360" height="46" viewbox="0 0 360 46"><defs><style type="text/css">#guide .c{fill:none;stroke:#222222;}#guide .k{fill:#000000;font-family:Roboto Mono,Sans-serif;font-size:20px;}#guide .s{fill:#e4f4ff;stroke:#222222;}#guide .syn{fill:#8D8D8D;font-family:Roboto Mono,Sans-serif;font-size:20px;}</style></defs><path class="c" d="M0 31h5m224 0h10m32 0h10m32 0h10m32 0h5"/><rect class="s" x="5" y="5" width="224" height="36"/><text class="k" x="15" y="31">TO_CARTESIANPOINT</text><rect class="s" x="239" y="5" width="32" height="36" rx="7"/><text class="syn" x="249" y="31">(</text><rect class="s" x="281" y="5" width="32" height="36" rx="7"/><text class="k" x="291" y="31">v</text><rect class="s" x="323" y="5" width="32" height="36" rx="7"/><text class="syn" x="333" y="31">)</text></svg>

+ 1 - 0
docs/reference/esql/functions/signature/to_cartesianshape.svg

@@ -0,0 +1 @@
+<svg version="1.1" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns="http://www.w3.org/2000/svg" width="360" height="46" viewbox="0 0 360 46"><defs><style type="text/css">#guide .c{fill:none;stroke:#222222;}#guide .k{fill:#000000;font-family:Roboto Mono,Sans-serif;font-size:20px;}#guide .s{fill:#e4f4ff;stroke:#222222;}#guide .syn{fill:#8D8D8D;font-family:Roboto Mono,Sans-serif;font-size:20px;}</style></defs><path class="c" d="M0 31h5m224 0h10m32 0h10m32 0h10m32 0h5"/><rect class="s" x="5" y="5" width="224" height="36"/><text class="k" x="15" y="31">TO_CARTESIANSHAPE</text><rect class="s" x="239" y="5" width="32" height="36" rx="7"/><text class="syn" x="249" y="31">(</text><rect class="s" x="281" y="5" width="32" height="36" rx="7"/><text class="k" x="291" y="31">v</text><rect class="s" x="323" y="5" width="32" height="36" rx="7"/><text class="syn" x="333" y="31">)</text></svg>

+ 1 - 0
docs/reference/esql/functions/signature/to_geopoint.svg

@@ -0,0 +1 @@
+<svg version="1.1" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns="http://www.w3.org/2000/svg" width="288" height="46" viewbox="0 0 288 46"><defs><style type="text/css">#guide .c{fill:none;stroke:#222222;}#guide .k{fill:#000000;font-family:Roboto Mono,Sans-serif;font-size:20px;}#guide .s{fill:#e4f4ff;stroke:#222222;}#guide .syn{fill:#8D8D8D;font-family:Roboto Mono,Sans-serif;font-size:20px;}</style></defs><path class="c" d="M0 31h5m152 0h10m32 0h10m32 0h10m32 0h5"/><rect class="s" x="5" y="5" width="152" height="36"/><text class="k" x="15" y="31">TO_GEOPOINT</text><rect class="s" x="167" y="5" width="32" height="36" rx="7"/><text class="syn" x="177" y="31">(</text><rect class="s" x="209" y="5" width="32" height="36" rx="7"/><text class="k" x="219" y="31">v</text><rect class="s" x="251" y="5" width="32" height="36" rx="7"/><text class="syn" x="261" y="31">)</text></svg>

+ 1 - 0
docs/reference/esql/functions/signature/to_geoshape.svg

@@ -0,0 +1 @@
+<svg version="1.1" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns="http://www.w3.org/2000/svg" width="288" height="46" viewbox="0 0 288 46"><defs><style type="text/css">#guide .c{fill:none;stroke:#222222;}#guide .k{fill:#000000;font-family:Roboto Mono,Sans-serif;font-size:20px;}#guide .s{fill:#e4f4ff;stroke:#222222;}#guide .syn{fill:#8D8D8D;font-family:Roboto Mono,Sans-serif;font-size:20px;}</style></defs><path class="c" d="M0 31h5m152 0h10m32 0h10m32 0h10m32 0h5"/><rect class="s" x="5" y="5" width="152" height="36"/><text class="k" x="15" y="31">TO_GEOSHAPE</text><rect class="s" x="167" y="5" width="32" height="36" rx="7"/><text class="syn" x="177" y="31">(</text><rect class="s" x="209" y="5" width="32" height="36" rx="7"/><text class="k" x="219" y="31">v</text><rect class="s" x="251" y="5" width="32" height="36" rx="7"/><text class="syn" x="261" y="31">)</text></svg>

+ 6 - 1
docs/reference/esql/functions/st_centroid.asciidoc

@@ -15,4 +15,9 @@ include::{esql-specs}/spatial.csv-spec[tag=st_centroid-airports-result]
 
 Supported types:
 
-include::types/st_centroid.asciidoc[]
+[%header.monospaced.styled,format=dsv,separator=|]
+|===
+v | result
+geo_point | geo_point
+cartesian_point | cartesian_point
+|===

+ 0 - 6
docs/reference/esql/functions/types/st_centroid.asciidoc

@@ -1,6 +0,0 @@
-[%header.monospaced.styled,format=dsv,separator=|]
-|===
-v | result
-geo_point | geo_point
-cartesian_point | cartesian_point
-|===

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

@@ -1088,7 +1088,7 @@ public abstract class AbstractFunctionTestCase extends ESTestCase {
             renderTypesTable(EsqlFunctionRegistry.description(definition).argNames());
             return;
         }
-        LogManager.getLogger(getTestClass()).info("Skipping rendering types because the function isn't registered");
+        LogManager.getLogger(getTestClass()).info("Skipping rendering types because the function '" + name + "' isn't registered");
     }
 
     private static void renderTypesTable(List<String> argNames) throws IOException {
@@ -1116,12 +1116,18 @@ public abstract class AbstractFunctionTestCase extends ESTestCase {
             [%header.monospaced.styled,format=dsv,separator=|]
             |===
             """ + header + "\n" + table.stream().collect(Collectors.joining("\n")) + "\n|===\n";
-        LogManager.getLogger(getTestClass()).info("Writing function types:\n{}", rendered);
+        LogManager.getLogger(getTestClass()).info("Writing function types for [{}]:\n{}", functionName(), rendered);
         writeToTempDir("types", rendered, "asciidoc");
     }
 
     private static String functionName() {
-        return StringUtils.camelCaseToUnderscore(getTestClass().getSimpleName().replace("Tests", "")).toLowerCase(Locale.ROOT);
+        Class<?> testClass = getTestClass();
+        if (testClass.isAnnotationPresent(FunctionName.class)) {
+            FunctionName functionNameAnnotation = testClass.getAnnotation(FunctionName.class);
+            return functionNameAnnotation.value();
+        } else {
+            return StringUtils.camelCaseToUnderscore(testClass.getSimpleName().replace("Tests", "")).toLowerCase(Locale.ROOT);
+        }
     }
 
     private static FunctionDefinition definition(String name) {
@@ -1178,6 +1184,7 @@ public abstract class AbstractFunctionTestCase extends ESTestCase {
         Files.createDirectories(dir);
         Path file = dir.resolve(functionName() + "." + extension);
         Files.writeString(file, str);
+        LogManager.getLogger(getTestClass()).info("Wrote function types for [{}] to file: {}", functionName(), file);
     }
 
     private final List<CircuitBreaker> breakers = Collections.synchronizedList(new ArrayList<>());

+ 30 - 0
x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/FunctionName.java

@@ -0,0 +1,30 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+package org.elasticsearch.xpack.esql.expression.function;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Tests that extend AbstractFunctionTestCase can use this annotation to specify the name of the function
+ * to use when generating documentation files while running tests.
+ * If this is not used, the name will be deduced from the test class name, by removing the "Test" suffix, and converting
+ * the class name to snake case. This annotation can be used to override that behavior, for cases where the deduced name
+ * is not correct. For example, in Elasticsearch the class name for `GeoPoint` capitalizes the `P` in `Point`, but the
+ * function name is `to_geopoint`, not `to_geo_point`. In some cases, even when compatible class names are used,
+ * like `StX` for the function `st_x`, the annotation is needed because the name deduction does not allow only a single
+ * character after the underscore.
+ */
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.TYPE)
+public @interface FunctionName {
+    /** The function name to use in generating documentation files while running tests */
+    String value();
+}

+ 2 - 0
x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToCartesianPointTests.java

@@ -13,6 +13,7 @@ import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.geo.ShapeTestUtils;
 import org.elasticsearch.xpack.esql.expression.function.AbstractFunctionTestCase;
+import org.elasticsearch.xpack.esql.expression.function.FunctionName;
 import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
 import org.elasticsearch.xpack.esql.type.EsqlDataTypes;
 import org.elasticsearch.xpack.ql.expression.Expression;
@@ -26,6 +27,7 @@ import java.util.function.Supplier;
 
 import static org.elasticsearch.xpack.ql.util.SpatialCoordinateTypes.CARTESIAN;
 
+@FunctionName("to_cartesianpoint")
 public class ToCartesianPointTests extends AbstractFunctionTestCase {
     public ToCartesianPointTests(@Name("TestCase") Supplier<TestCaseSupplier.TestCase> testCaseSupplier) {
         this.testCase = testCaseSupplier.get();

+ 2 - 0
x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToCartesianShapeTests.java

@@ -13,6 +13,7 @@ import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.geo.GeometryTestUtils;
 import org.elasticsearch.xpack.esql.expression.function.AbstractFunctionTestCase;
+import org.elasticsearch.xpack.esql.expression.function.FunctionName;
 import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
 import org.elasticsearch.xpack.esql.type.EsqlDataTypes;
 import org.elasticsearch.xpack.ql.expression.Expression;
@@ -26,6 +27,7 @@ import java.util.function.Supplier;
 
 import static org.elasticsearch.xpack.ql.util.SpatialCoordinateTypes.CARTESIAN;
 
+@FunctionName("to_cartesianshape")
 public class ToCartesianShapeTests extends AbstractFunctionTestCase {
     public ToCartesianShapeTests(@Name("TestCase") Supplier<TestCaseSupplier.TestCase> testCaseSupplier) {
         this.testCase = testCaseSupplier.get();

+ 2 - 0
x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToGeoPointTests.java

@@ -13,6 +13,7 @@ import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.geo.GeometryTestUtils;
 import org.elasticsearch.xpack.esql.expression.function.AbstractFunctionTestCase;
+import org.elasticsearch.xpack.esql.expression.function.FunctionName;
 import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
 import org.elasticsearch.xpack.esql.type.EsqlDataTypes;
 import org.elasticsearch.xpack.ql.expression.Expression;
@@ -26,6 +27,7 @@ import java.util.function.Supplier;
 
 import static org.elasticsearch.xpack.ql.util.SpatialCoordinateTypes.GEO;
 
+@FunctionName("to_geopoint")
 public class ToGeoPointTests extends AbstractFunctionTestCase {
     public ToGeoPointTests(@Name("TestCase") Supplier<TestCaseSupplier.TestCase> testCaseSupplier) {
         this.testCase = testCaseSupplier.get();

+ 2 - 0
x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToGeoShapeTests.java

@@ -13,6 +13,7 @@ import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.geo.GeometryTestUtils;
 import org.elasticsearch.xpack.esql.expression.function.AbstractFunctionTestCase;
+import org.elasticsearch.xpack.esql.expression.function.FunctionName;
 import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
 import org.elasticsearch.xpack.esql.type.EsqlDataTypes;
 import org.elasticsearch.xpack.ql.expression.Expression;
@@ -26,6 +27,7 @@ import java.util.function.Supplier;
 
 import static org.elasticsearch.xpack.ql.util.SpatialCoordinateTypes.GEO;
 
+@FunctionName("to_geoshape")
 public class ToGeoShapeTests extends AbstractFunctionTestCase {
     public ToGeoShapeTests(@Name("TestCase") Supplier<TestCaseSupplier.TestCase> testCaseSupplier) {
         this.testCase = testCaseSupplier.get();