Another set of fixes

elastic · Oct 15, 2024 · 7bca8dc · 7bca8dc
1 parent 0938d91
commit 7bca8dc
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 9 deletions.
diff --git a/...lugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/util/CollectionUtils.java b/...lugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/util/CollectionUtils.java
@@ -88,7 +88,7 @@ public static <T> List<T> nullSafeList(T... entries) {
         }
         List<T> list = new ArrayList<>(entries.length);
         for (T entry : entries) {
-            if (entry == null) {
+            if (entry != null) {
                 list.add(entry);
             }
         }

diff --git a/.../src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java b/.../src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java
@@ -485,11 +485,10 @@ public static DataType getTargetType(String[] names) {
     }
 
     public static FunctionDescription description(FunctionDefinition def) {
-        var constructors = def.clazz().getConstructors();
-        if (constructors.length == 0) {
+        Constructor<?> constructor = constructorFor(def.clazz());
+        if (constructor == null) {
             return new FunctionDescription(def.name(), List.of(), null, null, false, false);
         }
-        Constructor<?> constructor = constructors[0];
         FunctionInfo functionInfo = functionInfo(def);
         String functionDescription = functionInfo == null ? "" : functionInfo.description().replace('\n', ' ');
         String[] returnType = functionInfo == null ? new String[] { "?" } : removeUnderConstruction(functionInfo.returnType());
@@ -526,14 +525,29 @@ private static String[] removeUnderConstruction(String[] types) {
     }
 
     public static FunctionInfo functionInfo(FunctionDefinition def) {
-        var constructors = def.clazz().getConstructors();
-        if (constructors.length == 0) {
+        Constructor<?> constructor = constructorFor(def.clazz());
+        if (constructor == null) {
             return null;
         }
-        Constructor<?> constructor = constructors[0];
         return constructor.getAnnotation(FunctionInfo.class);
     }
 
+    private static Constructor<?> constructorFor(Class<? extends Function> clazz) {
+        Constructor<?>[] constructors = clazz.getConstructors();
+        if (constructors.length == 0) {
+            return null;
+        }
+        // when dealing with multiple, pick the constructor exposing the FunctionInfo annotation
+        if (constructors.length > 1) {
+            for (Constructor<?> constructor : constructors) {
+                if (constructor.getAnnotation(FunctionInfo.class) != null) {
+                    return constructor;
+                }
+            }
+        }
+        return constructors[0];
+    }
+
     private void buildDataTypesForStringLiteralConversion(FunctionDefinition[]... groupFunctions) {
         for (FunctionDefinition[] group : groupFunctions) {
             for (FunctionDefinition def : group) {

diff --git a/...k/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java b/...k/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java
@@ -328,7 +328,7 @@ public void testAggsWithGroupKeyAsAgg() throws Exception {
     public void testStatsWithGroupKeyAndAggFilter() throws Exception {
         var a = attribute("a");
         var f = new UnresolvedFunction(EMPTY, "min", DEFAULT, List.of(a));
-        var filter = new Alias(EMPTY, "min(a)", new FilteredExpression(EMPTY, f, new GreaterThan(EMPTY, a, integer(1))));
+        var filter = new Alias(EMPTY, "min(a) where a > 1", new FilteredExpression(EMPTY, f, new GreaterThan(EMPTY, a, integer(1))));
         assertEquals(
             new Aggregate(EMPTY, PROCESSING_CMD_INPUT, Aggregate.AggregateType.STANDARD, List.of(a), List.of(filter, a)),
             processingCommand("stats min(a) where a > 1 by a")
@@ -373,7 +373,7 @@ public void testStatsWithGroupKeyAndMixedAggAndFilter() throws Exception {
     public void testStatsWithoutGroupKeyMixedAggAndFilter() throws Exception {
         var a = attribute("a");
         var f = new UnresolvedFunction(EMPTY, "min", DEFAULT, List.of(a));
-        var filter = new Alias(EMPTY, "min(a)", new FilteredExpression(EMPTY, f, new GreaterThan(EMPTY, a, integer(1))));
+        var filter = new Alias(EMPTY, "min(a) where a > 1", new FilteredExpression(EMPTY, f, new GreaterThan(EMPTY, a, integer(1))));
         assertEquals(
             new Aggregate(EMPTY, PROCESSING_CMD_INPUT, Aggregate.AggregateType.STANDARD, List.of(), List.of(filter)),
             processingCommand("stats min(a) where a > 1")

diff --git a/...ck/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/tree/EsqlNodeSubclassTests.java b/...ck/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/tree/EsqlNodeSubclassTests.java
@@ -21,6 +21,8 @@
 import org.elasticsearch.xpack.esql.core.expression.Expression;
 import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
 import org.elasticsearch.xpack.esql.core.expression.Literal;
+import org.elasticsearch.xpack.esql.core.expression.MetadataAttribute;
+import org.elasticsearch.xpack.esql.core.expression.ReferenceAttribute;
 import org.elasticsearch.xpack.esql.core.expression.UnresolvedAttribute;
 import org.elasticsearch.xpack.esql.core.expression.UnresolvedAttributeTests;
 import org.elasticsearch.xpack.esql.core.expression.UnresolvedNamedExpression;
@@ -164,6 +166,17 @@ public void testInfoParameters() throws Exception {
          * in the parameters and not included.
          */
         expectedCount -= 1;
+
+        // special exceptions with private constructors
+        if (MetadataAttribute.class.equals(subclass) ||
+            ReferenceAttribute.class.equals(subclass)) {
+            expectedCount++;
+        }
+
+        if (FieldAttribute.class.equals(subclass)) {
+            expectedCount += 2;
+        }
+
         assertEquals(expectedCount, info(node).properties().size());
     }
 
@@ -174,6 +187,9 @@ public void testInfoParameters() throws Exception {
      * implementations in the process.
      */
     public void testTransform() throws Exception {
+        if (FieldAttribute.class.equals(subclass)) {
+            assumeTrue("FieldAttribute private constructor", false);
+        }
         Constructor<T> ctor = longestCtor(subclass);
         Object[] nodeCtorArgs = ctorArgs(ctor);
         T node = ctor.newInstance(nodeCtorArgs);