Skip to content

Commit 0ec1289

Browse files
chore(isthmus): remove dead code (#604)
Since the use of DdlSqlToRelConverter was added to SubstraitSqlToCalcite in commit 260a1c4, the original invocation of SqlToRelConverter that was left as a fallback when it returns null is never called. The reason is that DdlSqlToRelConverter itself already falls back to calling SqlToRelConverter so it never returns null. This means that removeRedundantProjects is never applied to the Rel structure since this only occurs in the uncalled fallback path. All tests continue to pass, which indicates that this optimiation is unnecessary. If it required in the future, it might be better written as follows to provide flexibility in the rules applied: static RelRoot applyPlannerRules(RelRoot root, RelOptRule... rules) { HepProgramBuilder builder = HepProgram.builder(); for (RelOptRule rule : rules) { builder.addRuleInstance(rule); } HepPlanner planner = new HepPlanner(builder.build()); planner.setRoot(root.rel); RelNode optimizedExpression = planner.findBestExp(); return root.withRel(optimizedExpression); } Signed-off-by: Mark S. Lewis <Mark.S.Lewis@outlook.com>
1 parent 442f1f2 commit 0ec1289

File tree

1 file changed

+1
-28
lines changed

1 file changed

+1
-28
lines changed

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

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import org.apache.calcite.prepare.Prepare;
1414
import org.apache.calcite.rel.RelNode;
1515
import org.apache.calcite.rel.RelRoot;
16-
import org.apache.calcite.rel.rules.CoreRules;
1716
import org.apache.calcite.rex.RexBuilder;
1817
import org.apache.calcite.sql.SqlNode;
1918
import org.apache.calcite.sql.parser.SqlParseException;
@@ -129,21 +128,9 @@ static List<RelRoot> convert(
129128
cluster,
130129
StandardConvertletTable.INSTANCE,
131130
SqlToRelConverter.CONFIG);
132-
// apply validation
133-
boolean needsValidation = true;
134-
// query is the root of the tree
135-
boolean top = true;
136131
DdlSqlToRelConverter ddlSqlToRelConverter = new DdlSqlToRelConverter(converter);
137132
return sqlNodes.stream()
138-
.map(
139-
sqlNode -> {
140-
RelRoot relRoot = sqlNode.accept(ddlSqlToRelConverter);
141-
if (relRoot == null) {
142-
relRoot =
143-
removeRedundantProjects(converter.convertQuery(sqlNode, needsValidation, top));
144-
}
145-
return relRoot;
146-
})
133+
.map(sqlNode -> sqlNode.accept(ddlSqlToRelConverter))
147134
.collect(Collectors.toList());
148135
}
149136

@@ -154,18 +141,4 @@ static RelOptCluster createDefaultRelOptCluster() {
154141
RelOptPlanner emptyPlanner = new HepPlanner(program);
155142
return RelOptCluster.create(emptyPlanner, rexBuilder);
156143
}
157-
158-
static RelRoot removeRedundantProjects(RelRoot root) {
159-
return root.withRel(removeRedundantProjects(root.rel));
160-
}
161-
162-
static RelNode removeRedundantProjects(RelNode root) {
163-
// The Calcite RelBuilder, when constructing Project that does not modify its inputs in any way,
164-
// simply elides it. The PROJECT_REMOVE rule can be used to remove such projects from Rel trees.
165-
// This facilitates roundtrip testing.
166-
HepProgram program = HepProgram.builder().addRuleInstance(CoreRules.PROJECT_REMOVE).build();
167-
HepPlanner planner = new HepPlanner(program);
168-
planner.setRoot(root);
169-
return planner.findBestExp();
170-
}
171144
}

0 commit comments

Comments
 (0)