refactor!: rename and move extension associated code (#148)

* refactor: rename FunctionSignatures to ExtensionSignatures
* refactor: rename FromProto to ProtoTypeConverter
* refactor: rename AbstractFunctionLookup to AbstractExtensionLookup
* refactor: rename FunctionLookup to ExtensionLookup
* refactor: rename ImmutableFunctionLookup to ImmutableExtensionLookup
* refactor: rename FunctionCollector to ExtensionCollector
* refactor: move extension related code to io.substrait.extension
* refactor: move AdvancedExtension to io.substrait.extension

---------

Co-authored-by: Jacques Nadeau <jacques@apache.org>

core/src/main/java/io/substrait/dsl/SubstraitBuilder.java

@@ -5,7 +5,7 @@
 import io.substrait.expression.Expression;
 import io.substrait.expression.FieldReference;
 import io.substrait.expression.ImmutableFieldReference;
-import io.substrait.function.SimpleExtension;
+import io.substrait.extension.SimpleExtension;
 import io.substrait.plan.ImmutablePlan;
 import io.substrait.plan.ImmutableRoot;
 import io.substrait.plan.Plan;

core/src/main/java/io/substrait/expression/AbstractFunctionInvocation.java

@@ -1,6 +1,6 @@
 package io.substrait.expression;
 
-import io.substrait.function.SimpleExtension;
+import io.substrait.extension.SimpleExtension;
 import io.substrait.type.Type;
 import java.util.List;
 

core/src/main/java/io/substrait/expression/AggregateFunctionInvocation.java

@@ -1,6 +1,6 @@
 package io.substrait.expression;
 
-import io.substrait.function.SimpleExtension;
+import io.substrait.extension.SimpleExtension;
 import io.substrait.proto.AggregateFunction;
 import io.substrait.type.Type;
 import java.util.List;

core/src/main/java/io/substrait/expression/EnumArg.java

@@ -1,6 +1,6 @@
 package io.substrait.expression;
 
-import io.substrait.function.SimpleExtension;
+import io.substrait.extension.SimpleExtension;
 import java.util.Optional;
 import org.immutables.value.Value;
 

core/src/main/java/io/substrait/expression/Expression.java

@@ -1,7 +1,7 @@
 package io.substrait.expression;
 
 import com.google.protobuf.ByteString;
-import io.substrait.function.SimpleExtension;
+import io.substrait.extension.SimpleExtension;
 import io.substrait.relation.Aggregate;
 import io.substrait.relation.Rel;
 import io.substrait.type.Type;

core/src/main/java/io/substrait/expression/ExpressionCreator.java

@@ -1,7 +1,7 @@
 package io.substrait.expression;
 
 import com.google.protobuf.ByteString;
-import io.substrait.function.SimpleExtension;
+import io.substrait.extension.SimpleExtension;
 import io.substrait.proto.AggregateFunction;
 import io.substrait.type.Type;
 import io.substrait.util.DecimalUtil;

core/src/main/java/io/substrait/expression/FunctionArg.java

@@ -1,11 +1,11 @@
 package io.substrait.expression;
 
 import io.substrait.expression.proto.ProtoExpressionConverter;
-import io.substrait.function.SimpleExtension;
+import io.substrait.extension.SimpleExtension;
 import io.substrait.function.TypeExpressionVisitor;
 import io.substrait.proto.FunctionArgument;
 import io.substrait.type.Type;
-import io.substrait.type.proto.FromProto;
+import io.substrait.type.proto.ProtoTypeConverter;
 
 /**
  * FunctionArg is a marker interface that represents an argument of a {@link
@@ -65,9 +65,10 @@ public FunctionArgument visitEnumArg(SimpleExtension.Function fnDef, int argIdx,
    */
   class ProtoFrom {
     private final ProtoExpressionConverter protoExprConverter;
-    private final FromProto protoTypeConverter;
+    private final ProtoTypeConverter protoTypeConverter;
 
-    public ProtoFrom(ProtoExpressionConverter protoExprConverter, FromProto protoTypeConverter) {
+    public ProtoFrom(
+        ProtoExpressionConverter protoExprConverter, ProtoTypeConverter protoTypeConverter) {
       this.protoExprConverter = protoExprConverter;
       this.protoTypeConverter = protoTypeConverter;
     }

core/src/main/java/io/substrait/expression/WindowFunctionInvocation.java

@@ -1,6 +1,6 @@
 package io.substrait.expression;
 
-import io.substrait.function.SimpleExtension;
+import io.substrait.extension.SimpleExtension;
 import io.substrait.proto.AggregateFunction;
 import io.substrait.type.Type;
 import java.util.List;

core/src/main/java/io/substrait/expression/proto/ExpressionProtoConverter.java

@@ -4,6 +4,7 @@
 import io.substrait.expression.FieldReference;
 import io.substrait.expression.FunctionArg;
 import io.substrait.expression.WindowBound;
+import io.substrait.extension.ExtensionCollector;
 import io.substrait.proto.Expression;
 import io.substrait.proto.Rel;
 import io.substrait.proto.SortField;
@@ -19,12 +20,12 @@ public class ExpressionProtoConverter implements ExpressionVisitor<Expression, R
   static final org.slf4j.Logger logger =
       org.slf4j.LoggerFactory.getLogger(ExpressionProtoConverter.class);
 
-  private final FunctionCollector extensionCollector;
+  private final ExtensionCollector extensionCollector;
   private final RelVisitor<Rel, RuntimeException> relVisitor;
   private final TypeProtoConverter typeProtoConverter;
 
   public ExpressionProtoConverter(
-      FunctionCollector extensionCollector, RelVisitor<Rel, RuntimeException> relVisitor) {
+      ExtensionCollector extensionCollector, RelVisitor<Rel, RuntimeException> relVisitor) {
     this.extensionCollector = extensionCollector;
     this.relVisitor = relVisitor;
     this.typeProtoConverter = new TypeProtoConverter(extensionCollector);

core/src/main/java/io/substrait/expression/proto/ProtoExpressionConverter.java

@@ -4,12 +4,12 @@
 import io.substrait.expression.ExpressionCreator;
 import io.substrait.expression.FieldReference;
 import io.substrait.expression.FunctionArg;
-import io.substrait.expression.FunctionLookup;
 import io.substrait.expression.ImmutableExpression;
-import io.substrait.function.SimpleExtension;
+import io.substrait.extension.ExtensionLookup;
+import io.substrait.extension.SimpleExtension;
 import io.substrait.relation.ProtoRelConverter;
 import io.substrait.type.Type;
-import io.substrait.type.proto.FromProto;
+import io.substrait.type.proto.ProtoTypeConverter;
 import java.util.ArrayList;
 import java.util.Objects;
 import java.util.stream.Collectors;
@@ -24,17 +24,19 @@ public class ProtoExpressionConverter {
 
   public static final Type.Struct EMPTY_TYPE = Type.Struct.builder().nullable(false).build();
 
-  private final FunctionLookup lookup;
+  private final ExtensionLookup lookup;
   private final SimpleExtension.ExtensionCollection extensions;
   private final Type.Struct rootType;
-  private final FromProto protoTypeConverter;
+  private final ProtoTypeConverter protoTypeConverter;
 
   public ProtoExpressionConverter(
-      FunctionLookup lookup, SimpleExtension.ExtensionCollection extensions, Type.Struct rootType) {
+      ExtensionLookup lookup,
+      SimpleExtension.ExtensionCollection extensions,
+      Type.Struct rootType) {
     this.lookup = lookup;
     this.extensions = extensions;
     this.rootType = Objects.requireNonNull(rootType, "rootType");
-    this.protoTypeConverter = new FromProto(lookup, extensions);
+    this.protoTypeConverter = new ProtoTypeConverter(lookup, extensions);
   }
 
   public FieldReference from(io.substrait.proto.Expression.FieldReference reference) {

core/src/main/java/io/substrait/expression/proto/AbstractFunctionLookup.java → core/src/main/java/io/substrait/extension/AbstractExtensionLookup.java

@@ -1,15 +1,12 @@
-package io.substrait.expression.proto;
+package io.substrait.extension;
 
-import io.substrait.expression.FunctionLookup;
-import io.substrait.function.SimpleExtension;
 import java.util.Map;
 
-public abstract class AbstractFunctionLookup implements FunctionLookup {
-  // TODO: Rename to AbstractExtensionLookup and move to io.substrait.extension
+public abstract class AbstractExtensionLookup implements ExtensionLookup {
   protected final Map<Integer, SimpleExtension.FunctionAnchor> functionAnchorMap;
   protected final Map<Integer, SimpleExtension.TypeAnchor> typeAnchorMap;
 
-  public AbstractFunctionLookup(
+  public AbstractExtensionLookup(
       Map<Integer, SimpleExtension.FunctionAnchor> functionAnchorMap,
       Map<Integer, SimpleExtension.TypeAnchor> typeAnchorMap) {
     this.functionAnchorMap = functionAnchorMap;

core/src/main/java/io/substrait/io/substrait/extension/AdvancedExtension.java → core/src/main/java/io/substrait/extension/AdvancedExtension.java

@@ -1,4 +1,4 @@
-package io.substrait.io.substrait.extension;
+package io.substrait.extension;
 
 import io.substrait.relation.Extension;
 import java.util.Optional;

core/src/main/java/io/substrait/expression/proto/FunctionCollector.java → core/src/main/java/io/substrait/extension/ExtensionCollector.java

@@ -1,6 +1,5 @@
-package io.substrait.expression.proto;
+package io.substrait.extension;
 
-import io.substrait.function.SimpleExtension;
 import io.substrait.proto.Plan;
 import io.substrait.proto.SimpleExtensionDeclaration;
 import io.substrait.proto.SimpleExtensionURI;
@@ -14,17 +13,17 @@
  * <p>Used to replace instances of function and types in the POJOs with references when converting
  * from {@link io.substrait.plan.Plan} to {@link io.substrait.proto.Plan}
  */
-public class FunctionCollector extends AbstractFunctionLookup {
-  // TODO: Rename to ExtensionCollector and move to io.substrait.extension
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(FunctionCollector.class);
+public class ExtensionCollector extends AbstractExtensionLookup {
+  static final org.slf4j.Logger logger =
+      org.slf4j.LoggerFactory.getLogger(ExtensionCollector.class);
 
   private final BidiMap<Integer, SimpleExtension.FunctionAnchor> funcMap;
   private final BidiMap<Integer, SimpleExtension.TypeAnchor> typeMap;
   private final BidiMap<Integer, String> uriMap;
 
   private int counter = -1;
 
-  public FunctionCollector() {
+  public ExtensionCollector() {
     super(new HashMap<>(), new HashMap<>());
     funcMap = new BidiMap<>(functionAnchorMap);
     typeMap = new BidiMap<>(typeAnchorMap);

core/src/main/java/io/substrait/expression/FunctionLookup.java → core/src/main/java/io/substrait/extension/ExtensionLookup.java

@@ -1,14 +1,11 @@
-package io.substrait.expression;
-
-import io.substrait.function.SimpleExtension;
+package io.substrait.extension;
 
 /**
  * Interface with operations for resolving references to {@link
  * io.substrait.proto.SimpleExtensionDeclaration}s within an individual plan to their corresponding
  * functions or types.
  */
-public interface FunctionLookup {
-  // TODO: Rename to ExtensionLookup and move to io.substrait.extension
+public interface ExtensionLookup {
   SimpleExtension.ScalarFunctionVariant getScalarFunction(
       int reference, SimpleExtension.ExtensionCollection extensions);
 

core/src/main/java/io/substrait/expression/proto/ImmutableFunctionLookup.java → core/src/main/java/io/substrait/extension/ImmutableExtensionLookup.java

@@ -1,6 +1,5 @@
-package io.substrait.expression.proto;
+package io.substrait.extension;
 
-import io.substrait.function.SimpleExtension;
 import io.substrait.proto.Plan;
 import io.substrait.proto.SimpleExtensionDeclaration;
 import java.util.Collections;
@@ -11,14 +10,13 @@
  * Maintains a mapping between function anchors and function references. Generates references for
  * new anchors.
  */
-public class ImmutableFunctionLookup extends AbstractFunctionLookup {
-  // TODO: Rename to ImmutableExtensionLookup and move to io.substrait.extension
+public class ImmutableExtensionLookup extends AbstractExtensionLookup {
   static final org.slf4j.Logger logger =
-      org.slf4j.LoggerFactory.getLogger(ImmutableFunctionLookup.class);
+      org.slf4j.LoggerFactory.getLogger(ImmutableExtensionLookup.class);
 
   private int counter = -1;
 
-  private ImmutableFunctionLookup(
+  private ImmutableExtensionLookup(
       Map<Integer, SimpleExtension.FunctionAnchor> functionMap,
       Map<Integer, SimpleExtension.TypeAnchor> typeMap) {
     super(functionMap, typeMap);
@@ -75,8 +73,8 @@ public Builder from(Plan p) {
       return this;
     }
 
-    public ImmutableFunctionLookup build() {
-      return new ImmutableFunctionLookup(
+    public ImmutableExtensionLookup build() {
+      return new ImmutableExtensionLookup(
           Collections.unmodifiableMap(functionMap), Collections.unmodifiableMap(typeMap));
     }
   }

core/src/main/java/io/substrait/function/SimpleExtension.java → core/src/main/java/io/substrait/extension/SimpleExtension.java

@@ -1,4 +1,4 @@
-package io.substrait.function;
+package io.substrait.extension;
 
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
@@ -12,6 +12,9 @@
 import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
 import com.fasterxml.jackson.datatype.jdk8.Jdk8Module;
 import io.substrait.expression.Expression;
+import io.substrait.function.ParameterizedType;
+import io.substrait.function.ToTypeString;
+import io.substrait.function.TypeExpression;
 import io.substrait.type.Deserializers;
 import io.substrait.type.TypeExpressionEvaluator;
 import io.substrait.util.Util;
@@ -28,7 +31,6 @@
 /** Classes used to deserialize YAML extension files. Handles functions and types. */
 @Value.Enclosing
 public class SimpleExtension {
-  // TODO: Move to io.substrait.extension
   static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(SimpleExtension.class);
 
   private static final ObjectMapper MAPPER =
@@ -552,12 +554,10 @@ public TypeAnchor getAnchor() {
         Util.memoize(() -> TypeAnchor.of(uri(), name()));
   }
 
-  @JsonDeserialize(as = ImmutableSimpleExtension.FunctionSignatures.class)
-  @JsonSerialize(as = ImmutableSimpleExtension.FunctionSignatures.class)
+  @JsonDeserialize(as = ImmutableSimpleExtension.ExtensionSignatures.class)
+  @JsonSerialize(as = ImmutableSimpleExtension.ExtensionSignatures.class)
   @Value.Immutable
-  public abstract static class FunctionSignatures {
-    // TODO: Rename to ExtensionSignatures ???
-
+  public abstract static class ExtensionSignatures {
     @JsonProperty("types")
     public abstract List<Type> types();
 
@@ -764,7 +764,7 @@ public static ExtensionCollection load(List<String> resourcePaths) {
 
   public static ExtensionCollection load(String namespace, String str) {
     try {
-      var doc = MAPPER.readValue(str, FunctionSignatures.class);
+      var doc = MAPPER.readValue(str, ExtensionSignatures.class);
       return buildExtensionCollection(namespace, doc);
     } catch (JsonProcessingException e) {
       throw new RuntimeException(e);
@@ -773,7 +773,7 @@ public static ExtensionCollection load(String namespace, String str) {
 
   public static ExtensionCollection load(String namespace, InputStream stream) {
     try {
-      var doc = MAPPER.readValue(stream, SimpleExtension.FunctionSignatures.class);
+      var doc = MAPPER.readValue(stream, ExtensionSignatures.class);
       return buildExtensionCollection(namespace, doc);
     } catch (RuntimeException ex) {
       throw ex;
@@ -783,7 +783,7 @@ public static ExtensionCollection load(String namespace, InputStream stream) {
   }
 
   public static ExtensionCollection buildExtensionCollection(
-      String namespace, SimpleExtension.FunctionSignatures extensionSignatures) {
+      String namespace, ExtensionSignatures extensionSignatures) {
     var collection =
         ImmutableSimpleExtension.ExtensionCollection.builder()
             .addAllAggregateFunctions(

core/src/main/java/io/substrait/plan/PlanProtoConverter.java

@@ -1,6 +1,6 @@
 package io.substrait.plan;
 
-import io.substrait.expression.proto.FunctionCollector;
+import io.substrait.extension.ExtensionCollector;
 import io.substrait.proto.Plan;
 import io.substrait.proto.PlanRel;
 import io.substrait.proto.Rel;
@@ -15,7 +15,7 @@ public class PlanProtoConverter {
 
   public Plan toProto(io.substrait.plan.Plan plan) {
     List<PlanRel> planRels = new ArrayList<>();
-    FunctionCollector functionCollector = new FunctionCollector();
+    ExtensionCollector functionCollector = new ExtensionCollector();
     for (io.substrait.plan.Plan.Root root : plan.getRoots()) {
       Rel input = new RelProtoConverter(functionCollector).toProto(root.getInput());
       planRels.add(

core/src/main/java/io/substrait/plan/ProtoPlanConverter.java

@@ -1,8 +1,8 @@
 package io.substrait.plan;
 
-import io.substrait.expression.FunctionLookup;
-import io.substrait.expression.proto.ImmutableFunctionLookup;
-import io.substrait.function.SimpleExtension;
+import io.substrait.extension.ExtensionLookup;
+import io.substrait.extension.ImmutableExtensionLookup;
+import io.substrait.extension.SimpleExtension;
 import io.substrait.proto.PlanRel;
 import io.substrait.relation.ProtoRelConverter;
 import io.substrait.relation.Rel;
@@ -27,12 +27,12 @@ public ProtoPlanConverter(SimpleExtension.ExtensionCollection extensionCollectio
   }
 
   /** Override hook for providing custom {@link ProtoRelConverter} implementations */
-  protected ProtoRelConverter getProtoRelConverter(FunctionLookup functionLookup) {
+  protected ProtoRelConverter getProtoRelConverter(ExtensionLookup functionLookup) {
     return new ProtoRelConverter(functionLookup, this.extensionCollection);
   }
 
   public Plan from(io.substrait.proto.Plan plan) {
-    FunctionLookup functionLookup = ImmutableFunctionLookup.builder().from(plan).build();
+    ExtensionLookup functionLookup = ImmutableExtensionLookup.builder().from(plan).build();
     ProtoRelConverter relConverter = getProtoRelConverter(functionLookup);
     List<Plan.Root> roots = new ArrayList<>();
     for (PlanRel planRel : plan.getRelationsList()) {

core/src/main/java/io/substrait/relation/HasExtension.java

@@ -1,6 +1,6 @@
 package io.substrait.relation;
 
-import io.substrait.io.substrait.extension.AdvancedExtension;
+import io.substrait.extension.AdvancedExtension;
 import java.util.Optional;
 
 /** Used to indicate the potential presence of an {@link AdvancedExtension} */