feat(core)!: derive container return types from nested argument bindings

core/src/main/java/io/substrait/extension/FunctionBindingResolver.java

@@ -27,14 +27,14 @@
  *
  * <p>Signature type matching is fail-closed. It checks value- and type-argument patterns alike:
  * wildcards, concrete types and the scalar-parameterized classes (decimal, char, binary, precision
- * time/timestamp, intervals); a declared shape carrying nested types (lists, maps, structs,
- * function types) is rejected rather than accepted unchecked. Occurrences of one numbered wildcard
- * ({@code any1}) must agree on a single type, while each plain {@code any} matches independently; a
- * variadic declaration repeats its trailing argument, requiring the repetitions to agree only when
- * its parameters are {@code CONSISTENT} — a literal integer parameter (the {@code 0} of {@code
- * DECIMAL<P,0>}) constrains every repetition regardless. Enum options and option preferences are
- * matched case-insensitively; an unspecified enum option is always rejected, since the extension
- * schema cannot declare an optional one.
+ * time/timestamp, intervals), and nested list, map, struct and function types. Nested structure and
+ * nullability must match; wildcard and integer parameters bind recursively. Occurrences of one
+ * numbered wildcard ({@code any1}) must agree on a single type, while each plain {@code any}
+ * matches independently; a variadic declaration repeats its trailing argument, requiring the
+ * repetitions to agree only when its parameters are {@code CONSISTENT} — a literal integer
+ * parameter (the {@code 0} of {@code DECIMAL<P,0>}) constrains every repetition regardless. Enum
+ * options and option preferences are matched case-insensitively; an unspecified enum option is
+ * always rejected, since the extension schema cannot declare an optional one.
  */
 public final class FunctionBindingResolver {
 
@@ -473,9 +473,12 @@ private static void requireKind(
   private static boolean typeMatches(
       ParameterizedType declared, Type actual, boolean exactNullability) {
     if (declared instanceof ParameterizedType.StringLiteral) {
-      // Non-wildcard extension parameter names at the top level are accepted; numbered wildcards
-      // are handled by the caller for cross-argument consistency.
-      return true;
+      // Top-level wildcards are handled by checkWildcard. Nested unmarked wildcards may bind a
+      // nullable type; an explicit '?' requires a nullable actual. The evaluator checks shared
+      // variable identities while deriving the return type, even when that return is concrete.
+      return !exactNullability
+          || !((ParameterizedType.StringLiteral) declared).nullable()
+          || actual.nullable();
     }
     if (declared instanceof Type) {
       // A concrete declared argument type (e.g. i32) matches ignoring nullability, except under a
@@ -520,15 +523,56 @@ private static boolean typeMatches(
       return actual instanceof Type.IntervalCompound
           && nullabilityMatches(declared, actual, exactNullability);
     }
-    // The remaining declared shapes — lists, maps, structs and function types — carry nested types
-    // this validator cannot yet check structurally, and the spec requires nested structure and
-    // nullability to match exactly: h(list<any1>, list<any1>) invoked as h(list<i32>, list<i32?>)
-    // must not bind (spec v0.99.0, scalar binding rules). A validator that advertises strictness
-    // must fail closed on a shape it cannot judge rather than silently accept it.
+    if (declared instanceof ParameterizedType.ListType) {
+      return actual instanceof Type.ListType
+          && nullabilityMatches(declared, actual, exactNullability)
+          && typeMatches(
+              ((ParameterizedType.ListType) declared).name(),
+              ((Type.ListType) actual).elementType(),
+              true);
+    }
+    if (declared instanceof ParameterizedType.Map) {
+      if (!(actual instanceof Type.Map)
+          || !nullabilityMatches(declared, actual, exactNullability)) {
+        return false;
+      }
+      ParameterizedType.Map pattern = (ParameterizedType.Map) declared;
+      Type.Map map = (Type.Map) actual;
+      return typeMatches(pattern.key(), map.key(), true)
+          && typeMatches(pattern.value(), map.value(), true);
+    }
+    if (declared instanceof ParameterizedType.Struct) {
+      return actual instanceof Type.Struct
+          && nullabilityMatches(declared, actual, exactNullability)
+          && typeListMatches(
+              ((ParameterizedType.Struct) declared).fields(), ((Type.Struct) actual).fields());
+    }
+    if (declared instanceof ParameterizedType.Func) {
+      if (!(actual instanceof Type.Func)
+          || !nullabilityMatches(declared, actual, exactNullability)) {
+        return false;
+      }
+      ParameterizedType.Func pattern = (ParameterizedType.Func) declared;
+      Type.Func function = (Type.Func) actual;
+      return typeListMatches(pattern.parameterTypes(), function.parameterTypes())
+          && typeMatches(pattern.returnType(), function.returnType(), true);
+    }
     throw new InvalidFunctionBindingException(
         String.format("Validation of the declared argument shape %s is not supported", declared));
   }
 
+  private static boolean typeListMatches(List<ParameterizedType> declared, List<Type> actual) {
+    if (declared.size() != actual.size()) {
+      return false;
+    }
+    for (int index = 0; index < declared.size(); index++) {
+      if (!typeMatches(declared.get(index), actual.get(index), true)) {
+        return false;
+      }
+    }
+    return true;
+  }
+
   /**
    * Under a DISCRETE declaration the declared nullability is part of the signature, for a
    * parameterized argument as much as for a concrete one. A declared shape that carries no

core/src/main/java/io/substrait/type/TypeExpressionEvaluator.java

@@ -1,15 +1,19 @@
 package io.substrait.type;
 
 import io.substrait.extension.SimpleExtension;
+import io.substrait.function.NullableType;
 import io.substrait.function.ParameterizedType;
 import io.substrait.function.TypeExpression;
 import io.substrait.function.TypeExpressionVisitor;
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.OptionalInt;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 /**
  * Evaluates a {@link TypeExpression} to a concrete {@link Type} given a set of actual arguments.
@@ -31,12 +35,12 @@
  * {@code interval_compound} at all, as an argument or as a return -- those two are supported for
  * symmetry, and pinned against hand-written declarations rather than the catalog.
  *
- * <p>A {@code list} return still fails whatever its element, because the evaluator does not descend
- * into a container -- so an element parameter it would otherwise substitute, as in {@code
- * list<varchar<L1>>}, is out of reach just as an element type to evaluate is. A multi-line return
- * program still fails because evaluating one needs integer arithmetic over the bound parameters
- * rather than substitution. And a plain {@code any} cannot be derived at all: unlike {@code any1}
- * it names nothing, so there is no identity to bind.
+ * <p>List, map, struct and function declarations bind their element, field, parameter and return
+ * types recursively. Container returns also evaluate their children, including integer parameters
+ * such as {@code List<varchar<L1>>} and type parameters such as {@code list<any1>}. Nested
+ * nullability is preserved; only the outermost argument nullability is excluded from wildcard
+ * identity. A multi-line return program still needs arithmetic rather than substitution. A plain
+ * {@code any} has no identity to bind, so it cannot be derived as a return type.
  *
  * <p>Which shipped variants those cover is pinned by {@code ParameterizedReturnTypeTest} against
  * the declarations the catalog ships, and deliberately not repeated here -- the catalog is owned
@@ -146,7 +150,7 @@ private static ParameterBindings bindParameters(
       }
       // An INCONSISTENT variadic repetition binds no named parameters — each repetition is
       // independent — but a literal constraint (the 0 of DECIMAL<P,0>) still applies to it.
-      bindings.bind(declared, actualTypes.get(index), !repeated || bindRepeats);
+      bindings.bind(declared, actualTypes.get(index), !repeated || bindRepeats, false);
     }
     return bindings;
   }
@@ -155,6 +159,7 @@ private static ParameterBindings bindParameters(
   private static final class ParameterBindings {
 
     private final Map<String, Type> types = new HashMap<>();
+    private final Set<String> exactTypeNullabilities = new HashSet<>();
     private final Map<String, Integer> integers = new HashMap<>();
 
     private Type boundType(String name) {
@@ -170,13 +175,29 @@ private Integer boundInteger(String token) {
      * an INCONSISTENT variadic repetition — named parameters are left unbound (each repetition is
      * independent) while literal constraints are still enforced.
      */
-    private void bind(ParameterizedType declared, Type actual, boolean bindNames) {
+    private void bind(ParameterizedType declared, Type actual, boolean bindNames, boolean nested) {
+      if (nested && !(declared instanceof ParameterizedType.StringLiteral)) {
+        if ((declared instanceof NullableType
+                && ((NullableType) declared).nullable() != actual.nullable())
+            || (declared instanceof Type && !declared.equals(actual))) {
+          throw cannotBind(declared, actual);
+        }
+      }
       if (declared instanceof ParameterizedType.StringLiteral) {
         ParameterizedType.StringLiteral literal = (ParameterizedType.StringLiteral) declared;
         // Only a numbered wildcard names a parameter that a return expression can refer to and that
         // has to stay consistent across the call; a plain "any" binds independently each time.
+        if (nested && literal.nullable() && !actual.nullable()) {
+          throw cannotBind(declared, actual);
+        }
         if (bindNames && literal.isNumberedWildcard()) {
-          bindType(literal.value(), actual);
+          // An unmarked nested wildcard binds the complete type, including nullability. A '?'
+          // marker requires a nullable actual, but does not constrain the variable's own
+          // nullability: both i32 and i32? become i32? after substitution. Outermost argument
+          // nullability is excluded from binding and also leaves the variable's nullability open.
+          boolean exactNullability = nested && !literal.nullable();
+          Type binding = nested && !literal.nullable() ? actual : actual.withNullable(false);
+          bindType(literal.value(), binding, exactNullability);
         }
       } else if (declared instanceof ParameterizedType.Decimal && actual instanceof Type.Decimal) {
         ParameterizedType.Decimal declaredDecimal = (ParameterizedType.Decimal) declared;
@@ -230,43 +251,71 @@ private void bind(ParameterizedType declared, Type actual, boolean bindNames) {
             ((ParameterizedType.IntervalCompound) declared).precision().value(),
             ((Type.IntervalCompound) actual).precision(),
             bindNames);
-      } else if (!(declared instanceof Type) && !isContainer(declared)) {
-        // A shape one of the arms above should have taken: the declaration carries a parameter and
-        // the actual type is not the class that would bind it. Binding nothing here would enforce
-        // the shared-parameter rule for some calls and skip it for others.
+      } else if (declared instanceof ParameterizedType.ListType
+          && actual instanceof Type.ListType) {
+        bind(
+            ((ParameterizedType.ListType) declared).name(),
+            ((Type.ListType) actual).elementType(),
+            bindNames,
+            true);
+      } else if (declared instanceof ParameterizedType.Map && actual instanceof Type.Map) {
+        ParameterizedType.Map pattern = (ParameterizedType.Map) declared;
+        Type.Map map = (Type.Map) actual;
+        bind(pattern.key(), map.key(), bindNames, true);
+        bind(pattern.value(), map.value(), bindNames, true);
+      } else if (declared instanceof ParameterizedType.Struct && actual instanceof Type.Struct) {
+        bindFields(
+            ((ParameterizedType.Struct) declared).fields(),
+            ((Type.Struct) actual).fields(),
+            bindNames);
+      } else if (declared instanceof ParameterizedType.Func && actual instanceof Type.Func) {
+        ParameterizedType.Func pattern = (ParameterizedType.Func) declared;
+        Type.Func function = (Type.Func) actual;
+        bindFields(pattern.parameterTypes(), function.parameterTypes(), bindNames);
+        bind(pattern.returnType(), function.returnType(), bindNames, true);
+      } else if (!(declared instanceof Type)) {
+        throw cannotBind(declared, actual);
+      }
+    }
+
+    private void bindFields(
+        List<ParameterizedType> declared, List<Type> actual, boolean bindNames) {
+      if (declared.size() != actual.size()) {
         throw new UnsupportedOperationException(
-            String.format(
-                "Cannot bind parameters from declared argument type %s to actual type %s",
-                declared, actual));
+            "Cannot bind container fields: expected "
+                + declared.size()
+                + " types but got "
+                + actual.size());
+      }
+      for (int index = 0; index < declared.size(); index++) {
+        bind(declared.get(index), actual.get(index), bindNames, true);
       }
     }
 
-    /**
-     * Whether the declared type holds other types rather than an integer parameter. Binding does
-     * not descend into these, so their parameters bind nothing and a mismatch cannot be told from a
-     * shape this method simply does not reach yet -- unlike the classes above, refusing here would
-     * reject declarations that resolve today without binding anything, such as a {@code list<any1>}
-     * argument to a function returning a concrete type.
-     *
-     * @param declared the declared argument type
-     * @return {@code true} if the type is a list, map, struct or function declaration
-     */
-    private boolean isContainer(ParameterizedType declared) {
-      return declared instanceof ParameterizedType.ListType
-          || declared instanceof ParameterizedType.Map
-          || declared instanceof ParameterizedType.Struct
-          || declared instanceof ParameterizedType.Func;
+    private static UnsupportedOperationException cannotBind(
+        ParameterizedType declared, Type actual) {
+      return new UnsupportedOperationException(
+          String.format(
+              "Cannot bind parameters from declared argument type %s to actual type %s",
+              declared, actual));
     }
 
-    private void bindType(String name, Type actual) {
-      // Nullability is not part of a wildcard's identity: any1 binds to i32 and i32? alike, and the
-      // return expression's own nullability (or the MIRROR policy) decides the result's.
+    private void bindType(String name, Type actual, boolean exactNullability) {
       Type existing = types.putIfAbsent(name, actual);
-      if (existing != null && !existing.equalsIgnoringNullability(actual)) {
+      boolean existingExact = exactTypeNullabilities.contains(name);
+      if (existing != null
+          && (!existing.equalsIgnoringNullability(actual)
+              || (existingExact && exactNullability && !existing.equals(actual)))) {
         throw new UnsupportedOperationException(
             String.format(
                 "Inconsistent binding for type parameter '%s': %s vs %s", name, existing, actual));
       }
+      if (exactNullability) {
+        exactTypeNullabilities.add(name);
+        if (!existingExact) {
+          types.put(name, actual);
+        }
+      }
     }
 
     private void bindInteger(String token, int value, boolean bindNames) {
@@ -374,18 +423,65 @@ public Type visit(ParameterizedType.IntervalCompound intervalCompound) {
           .intervalCompound(resolveInteger(intervalCompound.precision().value()));
     }
 
+    @Override
+    public Type visit(ParameterizedType.ListType list) {
+      return TypeCreator.of(list.nullable()).list(evaluateNested(list.name()));
+    }
+
+    @Override
+    public Type visit(ParameterizedType.Map map) {
+      return TypeCreator.of(map.nullable())
+          .map(evaluateNested(map.key()), evaluateNested(map.value()));
+    }
+
+    @Override
+    public Type visit(ParameterizedType.Struct struct) {
+      return TypeCreator.of(struct.nullable())
+          .struct(struct.fields().stream().map(this::evaluateNested).collect(Collectors.toList()));
+    }
+
+    @Override
+    public Type visit(ParameterizedType.Func function) {
+      return TypeCreator.of(function.nullable())
+          .func(
+              function.parameterTypes().stream()
+                  .map(this::evaluateNested)
+                  .collect(Collectors.toList()),
+              evaluateNested(function.returnType()));
+    }
+
+    private Type evaluateNested(ParameterizedType expression) {
+      if (expression instanceof Type) {
+        return (Type) expression;
+      }
+      if (expression instanceof ParameterizedType.StringLiteral) {
+        ParameterizedType.StringLiteral variable = (ParameterizedType.StringLiteral) expression;
+        Type bound = boundType(variable);
+        if (!variable.nullable() && !bindings.exactTypeNullabilities.contains(variable.value())) {
+          throw new UnsupportedOperationException(
+              "Cannot derive nullability of type parameter '" + variable.value() + "'");
+        }
+        return bound.withNullable(bound.nullable() || variable.nullable());
+      }
+      return expression.accept(this);
+    }
+
     @Override
     public Type visit(ParameterizedType.StringLiteral stringLiteral) {
       // A wildcard return (e.g. min(any1) -> any1) resolves to the bound argument type, taking the
       // nullability declared on the return expression in both directions (a required return forces
       // the type non-null, a nullable one forces it nullable). MIRROR policy, if any, is applied
       // afterwards by the caller.
+      return boundType(stringLiteral).withNullable(stringLiteral.nullable());
+    }
+
+    private Type boundType(ParameterizedType.StringLiteral stringLiteral) {
       Type bound = bindings.boundType(stringLiteral.value());
       if (bound == null) {
         throw new UnsupportedOperationException(
             "Unbound type parameter '" + stringLiteral.value() + "' in return-type expression");
       }
-      return bound.withNullable(stringLiteral.nullable());
+      return bound;
     }
 
     private int resolveInteger(String token) {

core/src/test/java/io/substrait/extension/FunctionBindingResolverTest.java

@@ -405,10 +405,17 @@ void inconsistentVariadicKeepsLiteralConstraints() {
   }
 
   @Test
-  void failsClosedOnANestedShapeItCannotCheck() {
+  void checksNestedShapeAndNullability() {
     SimpleExtension.ScalarFunctionVariant listPair = testScalar("list_pair:list_list");
-    // A declared list<any1> against a non-list actual used to be accepted silently; a strict
-    // validator must reject a shape it cannot check rather than pass it.
+    assertDoesNotThrow(
+        () ->
+            FunctionBindingResolver.resolveAndValidate(
+                listPair,
+                List.of(
+                    ResolvedArgument.value(R.list(N.I32)), ResolvedArgument.value(R.list(N.I32))),
+                List.of(),
+                R.BOOLEAN));
+    // The container shape must match before its element can bind.
     assertThrows(
         InvalidFunctionBindingException.class,
         () ->
@@ -417,8 +424,7 @@ void failsClosedOnANestedShapeItCannotCheck() {
                 List.of(ResolvedArgument.value(R.I32), ResolvedArgument.value(R.I32)),
                 List.of(),
                 R.BOOLEAN));
-    // list<i32> vs list<i32?> must not bind either: nested nullability is part of the structural
-    // match, which is exactly the check this validator cannot do yet — so it fails closed here too.
+    // Inner nullability is part of the shared wildcard binding.
     assertThrows(
         InvalidFunctionBindingException.class,
         () ->