zachjsh commented on code in PR #13627:
URL: https://github.com/apache/druid/pull/13627#discussion_r1067588648


##########
server/src/main/java/org/apache/druid/catalog/model/table/InputFormats.java:
##########
@@ -45,59 +42,85 @@
 import java.util.stream.Collectors;
 
 /**
- * Definition of the input formats which converts from property
- * lists in table specs to subclasses of {@link InputFormat}.
+ * Catalog definitions for the Druid input formats. Maps from catalog and SQL
+ * functions to the Druid input format implementations.
  */
 public class InputFormats
 {
-  public interface InputFormatDefn
-  {
-    String name();
-    String typeTag();
-    List<PropertyDefn<?>> properties();
-    void validate(ResolvedTable table);
-    InputFormat convert(ResolvedTable table);
-  }
-
+  /**
+   * Base class for input format definitions.
+   */
   public abstract static class BaseFormatDefn implements InputFormatDefn
   {
-    private final String name;
-    private final String typeTag;
-    private final List<PropertyDefn<?>> properties;
-
-    public BaseFormatDefn(
-        final String name,
-        final String typeTag,
-        final List<PropertyDefn<?>> properties
-    )
+    /**
+     * The set of SQL function parameters available when the format is
+     * specified via a SQL function. The parameters correspond to input format
+     * properties, but typically have simpler names and must require only 
simple
+     * scalar types of the kind that SQL can provide. Each subclass must 
perform
+     * conversion to the type required for Jackson conversion.
+     */
+    private final List<ParameterDefn> parameters;
+
+    public BaseFormatDefn(List<ParameterDefn> parameters)
     {
-      this.name = name;
-      this.typeTag = typeTag;
-      this.properties = properties;
+      this.parameters = parameters == null ? Collections.emptyList() : 
parameters;
     }
 
     @Override
-    public String name()
+    public List<ParameterDefn> parameters()
     {
-      return name;
+      return parameters;
     }
 
+    /**
+     * The target input format class for Jackson conversions.
+     */
+    protected abstract Class<? extends InputFormat> inputFormatClass();
+
     @Override
-    public String typeTag()
+    public void validate(ResolvedExternalTable table)
     {
-      return typeTag;
+      // Bare-bones validation: the format has to convert to the proper object.
+      // Subclasses should replace this with something fancier where needed.
+
+      if (table.inputFormatMap != null) {
+        convertFromTable(table);
+      }
     }
 
-    @Override
-    public List<PropertyDefn<?>> properties()
+    /**
+     * Convert columns from the {@link ColumnSpec} format used by the catalog 
to the
+     * list of names form requires by input formats.
+     */
+    protected void convertColumns(Map<String, Object> jsonMap, 
List<ColumnSpec> columns)
     {
-      return properties;
+      List<String> cols = columns
+          .stream()
+          .map(col -> col.name())
+          .collect(Collectors.toList());
+      jsonMap.put("columns", cols);

Review Comment:
   why are columns part of the inputFormat / why are they being mucked with 
here?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to