zabetak commented on code in PR #2935:
URL: https://github.com/apache/calcite/pull/2935#discussion_r992121760
##########
core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java:
##########
@@ -540,4 +540,32 @@ public interface JoinRightEmptyRuleConfig extends
PruneEmptyRule.Config {
};
}
}
+
+
+ public static final RelOptRule EMPTY_TABLE =
+ ImmutableEmptyTableOptimizationConfig.of()
+ .withOperandSupplier(b0 -> {
+ return b0.operand(TableScan.class).noInputs();
+ })
+ .withDescription("ConvertEmptyTableToValues")
Review Comment:
nit: To keep things uniform maybe rename to `"PruneZeroRowsTable"`
##########
core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java:
##########
@@ -540,4 +540,32 @@ public interface JoinRightEmptyRuleConfig extends
PruneEmptyRule.Config {
};
}
}
+
+
+ public static final RelOptRule EMPTY_TABLE =
+ ImmutableEmptyTableOptimizationConfig.of()
+ .withOperandSupplier(b0 -> {
+ return b0.operand(TableScan.class).noInputs();
+ })
+ .withDescription("ConvertEmptyTableToValues")
+ .toRule();
+
+ /** Configuration for rule that transforms an empty table into an empty
values node.
+ * MaxRowCount is used as the stat to transform, hence Table implementer
needs
+ * to supply this metadata if this optimization needs to be applied.*/
+ @Value.Immutable
+ public interface EmptyTableOptimizationConfig extends PruneEmptyRule.Config {
+
+ @Override default PruneEmptyRule toRule() {
+ return new PruneEmptyRule(this) {
+ @Override public void onMatch(RelOptRuleCall call) {
+ TableScan tableScan = call.rel(0);
+ Double maxRowCount =
call.getMetadataQuery().getMaxRowCount(tableScan);
+ if (maxRowCount != null && maxRowCount == 0.0) {
+ call.transformTo(call.builder().push(tableScan).empty().build());
Review Comment:
The effect of this rule is very similar to what `RemoveEmptySingleRule` is
doing. The latter is also trying to conserve the traits so maybe it is a good
idea to attemp to conserve the traits as well here.
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -3413,6 +3413,54 @@ RelOptFixture checkDynamicFunctions(boolean
treatDynamicCallsAsConstant) {
.check();
}
+ @Test void testEmptyTableProject() {
+ // table is transformed to empty values and extra project will be removed.
+ final String sql = "select * from EMPTY_PRODUCTS\n";
+ sql(sql)
+ .withRule(
+ PruneEmptyRules.EMPTY_TABLE,
+ PruneEmptyRules.PROJECT_INSTANCE)
+ .check();
+ }
+
+ @Test void testEmptyTableJoinLeft() {
+ // inner join is eliminated after the left table is transformed to empty
values.
+ final String sql = "select * from EMPTY_PRODUCTS as e\n"
+ + ",products as d where e.PRODUCTID = d.PRODUCTID\n";
+ sql(sql)
+ .withRule(
+ PruneEmptyRules.EMPTY_TABLE,
+ PruneEmptyRules.JOIN_LEFT_INSTANCE)
+ .check();
+ }
+
+ @Test void testEmptyTableJoinRight() {
+ // inner join is eliminated after the right table is transformed to empty
values.
+ final String sql = "select * from products as e\n"
+ + ",EMPTY_PRODUCTS as d where e.PRODUCTID = d.PRODUCTID\n";
+ sql(sql)
+ .withRule(
+ PruneEmptyRules.EMPTY_TABLE,
+ PruneEmptyRules.JOIN_RIGHT_INSTANCE)
+ .check();
+ }
Review Comment:
I don't think need a test case for every possible combination of rules of
`EMPTY_TABLE` with the others. I think we can keep just two:
* `testEmptyTable` which just tests the new rule.
* `testEmptyTableInComplexQuery` which exploits multiple pruning rules
including `EMPTY_TABLE` to simplify a plan completely leaving only an empty
`LogicalValues` at the end.
##########
core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java:
##########
@@ -540,4 +540,32 @@ public interface JoinRightEmptyRuleConfig extends
PruneEmptyRule.Config {
};
}
}
+
+
+ public static final RelOptRule EMPTY_TABLE =
Review Comment:
I think it would be a good idea to add the new rule to
`org.apache.calcite.plan.RelOptRules#ABSTRACT_RULES` list along with the other
pruning rules.
##########
core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java:
##########
@@ -540,4 +540,32 @@ public interface JoinRightEmptyRuleConfig extends
PruneEmptyRule.Config {
};
}
}
+
+
+ public static final RelOptRule EMPTY_TABLE =
+ ImmutableEmptyTableOptimizationConfig.of()
+ .withOperandSupplier(b0 -> {
+ return b0.operand(TableScan.class).noInputs();
+ })
+ .withDescription("ConvertEmptyTableToValues")
+ .toRule();
+
+ /** Configuration for rule that transforms an empty table into an empty
values node.
+ * MaxRowCount is used as the stat to transform, hence Table implementer
needs
+ * to supply this metadata if this optimization needs to be applied.*/
+ @Value.Immutable
+ public interface EmptyTableOptimizationConfig extends PruneEmptyRule.Config {
+
+ @Override default PruneEmptyRule toRule() {
+ return new PruneEmptyRule(this) {
+ @Override public void onMatch(RelOptRuleCall call) {
+ TableScan tableScan = call.rel(0);
+ Double maxRowCount =
call.getMetadataQuery().getMaxRowCount(tableScan);
+ if (maxRowCount != null && maxRowCount == 0.0) {
Review Comment:
It is better to put the row count check in the `matches` method. When the
condition is not satisfied the rule can be eliminated much earlier.
##########
testkit/src/main/java/org/apache/calcite/test/catalog/MockCatalogReader.java:
##########
@@ -618,6 +621,57 @@ public StructKind getKind() {
}
}
+ /**
+ * Mock implementation of
+ * {@link org.apache.calcite.prepare.Prepare.PreparingTable} which supplies
MaxRow stat.
+ */
+ public static class MaxRowMockTable
+ extends MockTable implements BuiltInMetadata.MaxRowCount.Handler {
Review Comment:
Maybe you can avoid introducing a new sub-class and make `MockTable`
implement the `MaxRowCount.Handler` assuming that rowCount == maxRowCount at
this point. If we need to differentiate in the future we can add or modify the
constructors/factories in `MockTable`.
##########
core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java:
##########
@@ -469,7 +470,6 @@ public interface SortFetchZeroRuleConfig extends
PruneEmptyRule.Config {
call.transformTo(emptyValues);
}
}
-
Review Comment:
nit: Please avoid unnecessary formatting changes.
##########
core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java:
##########
@@ -540,4 +540,32 @@ public interface JoinRightEmptyRuleConfig extends
PruneEmptyRule.Config {
};
}
}
+
+
+ public static final RelOptRule EMPTY_TABLE =
+ ImmutableEmptyTableOptimizationConfig.of()
+ .withOperandSupplier(b0 -> {
+ return b0.operand(TableScan.class).noInputs();
+ })
+ .withDescription("ConvertEmptyTableToValues")
+ .toRule();
+
+ /** Configuration for rule that transforms an empty table into an empty
values node.
+ * MaxRowCount is used as the stat to transform, hence Table implementer
needs
+ * to supply this metadata if this optimization needs to be applied.*/
+ @Value.Immutable
+ public interface EmptyTableOptimizationConfig extends PruneEmptyRule.Config {
Review Comment:
A possibly better name would be `ZeroMaxRowsRuleConfig`.
##########
core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java:
##########
@@ -540,4 +540,32 @@ public interface JoinRightEmptyRuleConfig extends
PruneEmptyRule.Config {
};
}
}
+
+
+ public static final RelOptRule EMPTY_TABLE =
+ ImmutableEmptyTableOptimizationConfig.of()
+ .withOperandSupplier(b0 -> {
+ return b0.operand(TableScan.class).noInputs();
+ })
Review Comment:
I think it can be simplified to `.withOperandSupplier(b0 ->
b0.operand(TableScan.class).noInputs());`
##########
core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java:
##########
@@ -540,4 +540,32 @@ public interface JoinRightEmptyRuleConfig extends
PruneEmptyRule.Config {
};
}
}
+
+
+ public static final RelOptRule EMPTY_TABLE =
+ ImmutableEmptyTableOptimizationConfig.of()
+ .withOperandSupplier(b0 -> {
+ return b0.operand(TableScan.class).noInputs();
+ })
+ .withDescription("ConvertEmptyTableToValues")
+ .toRule();
+
+ /** Configuration for rule that transforms an empty table into an empty
values node.
+ * MaxRowCount is used as the stat to transform, hence Table implementer
needs
+ * to supply this metadata if this optimization needs to be applied.*/
+ @Value.Immutable
+ public interface EmptyTableOptimizationConfig extends PruneEmptyRule.Config {
+
+ @Override default PruneEmptyRule toRule() {
+ return new PruneEmptyRule(this) {
+ @Override public void onMatch(RelOptRuleCall call) {
+ TableScan tableScan = call.rel(0);
Review Comment:
It is better to be more general here (use `RelNode` here instead of
`TableScan`); the rule will still work correctly and if people want to reduce
or enlarge the scope they can do it by changing the matching operands of the
configuration.
##########
core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java:
##########
@@ -540,4 +540,32 @@ public interface JoinRightEmptyRuleConfig extends
PruneEmptyRule.Config {
};
}
}
+
+
+ public static final RelOptRule EMPTY_TABLE =
Review Comment:
All other rules in this class are suffixed with `INSTANCE`. To keep things
uniform I would suggest one of the following:
* `EMPTY_TABLE_INSTANCE`
* `TABLE_ZERO_ROWS_INSTANCE`
* `ZERO_ROWS_TABLE_INSTANCE`
--
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]
