You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2020/04/19 23:52:41 UTC
[GitHub] [calcite] hbtoo opened a new pull request #1927: [CALCITE-3939] more auto pruning rules after SubstitutionRule is introduced
hbtoo opened a new pull request #1927:
URL: https://github.com/apache/calcite/pull/1927
https://issues.apache.org/jira/browse/CALCITE-3939
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] hsyuan commented on a change in pull request #1927: [CALCITE-3939] Change UnionEliminatorRule and ProjectRemoveRule to auto pruning SubstitutionRule
Posted by GitBox <gi...@apache.org>.
hsyuan commented on a change in pull request #1927:
URL: https://github.com/apache/calcite/pull/1927#discussion_r411856791
##########
File path: core/src/main/java/org/apache/calcite/rel/rules/UnionEliminatorRule.java
##########
@@ -44,18 +46,17 @@ public UnionEliminatorRule(Class<? extends Union> clazz,
//~ Methods ----------------------------------------------------------------
- public void onMatch(RelOptRuleCall call) {
+ @Override public boolean matches(RelOptRuleCall call) {
Union union = call.rel(0);
- if (union.getInputs().size() != 1) {
- return;
- }
- if (!union.all) {
- return;
- }
-
- // REVIEW jvs 14-Mar-2006: why don't we need to register
- // the equivalence here like we do in AggregateRemoveRule?
+ return union.all && union.getInputs().size() == 1;
Review comment:
I am curious when can we get a union with only 1 child?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] hbtoo commented on a change in pull request #1927: [CALCITE-3939] more auto pruning rules after SubstitutionRule is introduced
Posted by GitBox <gi...@apache.org>.
hbtoo commented on a change in pull request #1927:
URL: https://github.com/apache/calcite/pull/1927#discussion_r411584190
##########
File path: core/src/main/java/org/apache/calcite/rel/rules/ProjectRemoveRule.java
##########
@@ -88,7 +88,6 @@ public static boolean isTrivial(Project project) {
project.getInput().getRowType());
}
- @Deprecated // to be removed before 1.5
public static boolean isIdentity(List<? extends RexNode> exps,
RelDataType childRowType) {
Review comment:
I am only testing the continuous testing failure with a trivial change. It seems irrelevant to my patch. I've just pushed the real change.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] hbtoo commented on a change in pull request #1927: [CALCITE-3939] Change UnionEliminatorRule and ProjectRemoveRule to auto pruning SubstitutionRule
Posted by GitBox <gi...@apache.org>.
hbtoo commented on a change in pull request #1927:
URL: https://github.com/apache/calcite/pull/1927#discussion_r412360659
##########
File path: core/src/main/java/org/apache/calcite/rel/rules/UnionEliminatorRule.java
##########
@@ -44,18 +46,17 @@ public UnionEliminatorRule(Class<? extends Union> clazz,
//~ Methods ----------------------------------------------------------------
- public void onMatch(RelOptRuleCall call) {
+ @Override public boolean matches(RelOptRuleCall call) {
Union union = call.rel(0);
- if (union.getInputs().size() != 1) {
- return;
- }
- if (!union.all) {
- return;
- }
-
- // REVIEW jvs 14-Mar-2006: why don't we need to register
- // the equivalence here like we do in AggregateRemoveRule?
+ return union.all && union.getInputs().size() == 1;
Review comment:
I guess one possible case is this. Unions are typically created via:
public static LogicalUnion create(List<RelNode> inputs, boolean all);
"List<RelNode> inputs" is typically assembled with a for loop. Whenever the for loop executes with only one loop, we have an input list of one element.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] chunweilei commented on a change in pull request #1927: [CALCITE-3939] more auto pruning rules after SubstitutionRule is introduced
Posted by GitBox <gi...@apache.org>.
chunweilei commented on a change in pull request #1927:
URL: https://github.com/apache/calcite/pull/1927#discussion_r411108717
##########
File path: core/src/main/java/org/apache/calcite/rel/rules/ProjectRemoveRule.java
##########
@@ -88,7 +88,6 @@ public static boolean isTrivial(Project project) {
project.getInput().getRowType());
}
- @Deprecated // to be removed before 1.5
public static boolean isIdentity(List<? extends RexNode> exps,
RelDataType childRowType) {
Review comment:
I don't understand what you want to do. Could you explain more about it?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org