chore(isthmus): rebase + refactoring

Author: ZorinAnton

isthmus/src/main/java/io/substrait/isthmus/SqlToSubstrait.java

@@ -1,25 +1,14 @@
 package io.substrait.isthmus;
 
-import com.google.common.annotations.VisibleForTesting;
-import io.substrait.isthmus.expression.DdlRelBuilder;
-import io.substrait.isthmus.sql.SubstraitSqlValidator;
+import io.substrait.isthmus.operation.CalciteOperation;
+import io.substrait.isthmus.sql.SubstraitSqlToCalcite;
 import io.substrait.plan.ImmutablePlan.Builder;
 import io.substrait.plan.Plan;
 import io.substrait.plan.Plan.Version;
 import io.substrait.plan.PlanProtoConverter;
 import java.util.List;
-import org.apache.calcite.plan.hep.HepPlanner;
-import org.apache.calcite.plan.hep.HepProgram;
 import org.apache.calcite.prepare.Prepare;
-import org.apache.calcite.rel.RelRoot;
-import org.apache.calcite.rel.rules.CoreRules;
-import org.apache.calcite.sql.SqlNode;
-import org.apache.calcite.sql.SqlNodeList;
 import org.apache.calcite.sql.parser.SqlParseException;
-import org.apache.calcite.sql.parser.SqlParser;
-import org.apache.calcite.sql.validate.SqlValidator;
-import org.apache.calcite.sql2rel.SqlToRelConverter;
-import org.apache.calcite.sql2rel.StandardConvertletTable;
 
 /** Take a SQL statement and a set of table definitions and return a substrait plan. */
 public class SqlToSubstrait extends SqlConverterBase {
@@ -66,126 +55,14 @@ public Plan convert(String sqlStatements, Prepare.CatalogReader catalogReader)
     builder.version(Version.builder().from(Version.DEFAULT_VERSION).producer("isthmus").build());
 
     // TODO: consider case in which one sql passes conversion while others don't
-    sqlToRelNode(sqlStatements, catalogReader).stream()
-        .map(root -> SubstraitRelVisitor.convert(root, EXTENSION_COLLECTION, featureBoard))
-        .forEach(root -> builder.addRoots(root));
-
-    return builder.build();
-  }
-
-    /**
-     * Converts one or more SQL statements into a Substrait {@link Plan}.
-     *
-     * @param sqlStatements a string containing one more SQL statements
-     * @param catalogReader the {@link Prepare.CatalogReader} for finding tables/views referenced in
-     *     the SQL statements
-     * @return the Substrait {@link Plan}
-     * @throws SqlParseException if there is an error while parsing the SQL statements
-     */
-    public Plan convertNew(String sqlStatements, Prepare.CatalogReader catalogReader)
-            throws SqlParseException {
-        Builder builder = io.substrait.plan.Plan.builder();
-        builder.version(Version.builder().from(Version.DEFAULT_VERSION).producer("isthmus").build());
-
-        // TODO: consider case in which one sql passes conversion while others don't
-        SubstraitSqlToCalcite.convertQueries(sqlStatements, catalogReader).stream()
-                .map(root -> SubstraitRelVisitor.convert(root, EXTENSION_COLLECTION, featureBoard))
-                .forEach(root -> builder.addRoots(root));
-
-        return builder.build();
-    }
-
-  @VisibleForTesting
-  List<RelRoot> sqlToRelNode(String sql, Prepare.CatalogReader catalogReader)
-      throws SqlParseException {
-    SqlValidator validator = new SubstraitSqlValidator(catalogReader);
-    SqlParser parser = SqlParser.create(sql, parserConfig);
-    SqlNodeList parsedList = parser.parseStmtList();
-    SqlToRelConverter converter = createSqlToRelConverter(validator, catalogReader);
-    List<RelRoot> roots =
-        parsedList.stream()
-            .map(parsed -> getBestExpRelRoot(converter, parsed))
-            .collect(java.util.stream.Collectors.toList());
-    return roots;
-  }
-
-  void sqlToPlanRoots(
-      String sql, SqlValidator validator, Prepare.CatalogReader catalogReader, Builder builder)
-      throws SqlParseException {
-    List<CalciteOperation> calciteOperations = sqlToCalciteOperation(sql, validator, catalogReader);
+    List<CalciteOperation> calciteOperations =
+        SubstraitSqlToCalcite.convertQueries(sqlStatements, catalogReader);
     CalciteOperationToSubstrait calciteOperationToSubstrait =
         new CalciteOperationToSubstrait(EXTENSION_COLLECTION, featureBoard);
     calciteOperations.stream()
         .map(operation -> operation.accept(calciteOperationToSubstrait))
         .forEach(builder::addRoots);
-  }
-
-  void sqlToPlanRoots1(
-      String sql, SqlValidator validator, Prepare.CatalogReader catalogReader, Builder builder)
-      throws SqlParseException {
-
-    SqlParser parser = SqlParser.create(sql, parserConfig);
-    SqlNodeList parsedList = parser.parseStmtList();
-    if (parsedList.isEmpty()) {
-      return;
-    }
-
-    SqlToRelConverter converter = createSqlToRelConverter(validator, catalogReader);
-    DdlRelBuilder ddlRelBuilder =
-        new DdlRelBuilder(
-            converter, SqlToSubstrait::getBestExpRelRoot, EXTENSION_COLLECTION, featureBoard);
-
-    List<SqlNode> nonDdlNodes = new ArrayList<>();
 
-    for (SqlNode sqlNode : parsedList) {
-      final io.substrait.plan.Plan.Root ddlRoot = sqlNode.accept(ddlRelBuilder);
-      if (ddlRoot != null) {
-        builder.addRoots(ddlRoot);
-      } else {
-        nonDdlNodes.add(sqlNode);
-      }
-    }
-
-    if (!nonDdlNodes.isEmpty()) {
-      SqlNodeList dmlNodes = new SqlNodeList(nonDdlNodes, parsedList.getParserPosition());
-
-      List<RelRoot> relRoots = sqlNodesToRelNode(dmlNodes, converter);
-      relRoots.stream()
-          .map(root -> SubstraitRelVisitor.convert(root, EXTENSION_COLLECTION, featureBoard))
-          .forEach(builder::addRoots);
-    }
-  }
-
-  private List<RelRoot> sqlNodesToRelNode(
-      final SqlNodeList parsedList, final SqlToRelConverter converter) {
-    return parsedList.stream()
-        .map(parsed -> getBestExpRelRoot(converter, parsed))
-        .collect(java.util.stream.Collectors.toList());
-  }
-
-  protected SqlToRelConverter createSqlToRelConverter(
-      SqlValidator validator, Prepare.CatalogReader catalogReader) {
-    SqlToRelConverter converter =
-        new SqlToRelConverter(
-            null,
-            validator,
-            catalogReader,
-            relOptCluster,
-            StandardConvertletTable.INSTANCE,
-            converterConfig);
-    return converter;
-  }
-
-  protected static RelRoot getBestExpRelRoot(SqlToRelConverter converter, SqlNode parsed) {
-    RelRoot root = converter.convertQuery(parsed, true, true);
-    {
-      // RelBuilder seems to implicitly use the rule below,
-      // need to add to avoid discrepancies in assertFullRoundTrip
-      HepProgram program = HepProgram.builder().addRuleInstance(CoreRules.PROJECT_REMOVE).build();
-      HepPlanner hepPlanner = new HepPlanner(program);
-      hepPlanner.setRoot(root.rel);
-      root = root.withRel(hepPlanner.findBestExp());
-    }
-    return root;
+    return builder.build();
   }
 }

isthmus/src/main/java/io/substrait/isthmus/sql/SubstraitSqlStatementParser.java

@@ -31,4 +31,18 @@ public static List<SqlNode> parseStatements(String sqlStatements) throws SqlPars
     SqlParser parser = SqlParser.create(sqlStatements, PARSER_CONFIG);
     return parser.parseStmtList();
   }
+
+  /**
+   * Parse one or more SQL statements to a list of {@link SqlNode}s.
+   *
+   * @param sqlStatements a string containing one or more SQL statements
+   * @param parserConfig sql parser configuration
+   * @return a list of {@link SqlNode}s corresponding to the given statements
+   * @throws SqlParseException if there is an error while parsing the SQL statements
+   */
+  public static List<SqlNode> parseStatements(String sqlStatements, SqlParser.Config parserConfig)
+      throws SqlParseException {
+    SqlParser parser = SqlParser.create(sqlStatements, parserConfig);
+    return parser.parseStmtList();
+  }
 }