zabetak commented on a change in pull request #1130: [CALCITE-2865]
FilterProjectTransposeRule generates wrong traitSet when copyFilter/Project is
true
URL: https://github.com/apache/calcite/pull/1130#discussion_r273732884
##########
File path:
core/src/main/java/org/apache/calcite/rel/rules/FilterProjectTransposeRule.java
##########
@@ -155,8 +158,12 @@ public void onMatch(RelOptRuleCall call) {
final RelBuilder relBuilder = call.builder();
RelNode newFilterRel;
if (copyFilter) {
- newFilterRel = filter.copy(filter.getTraitSet(), project.getInput(),
- simplifyFilterCondition(newCondition, call));
+ final RelNode input = project.getInput();
+ final RelTraitSet traitSet = filter.getTraitSet()
+ .replaceIfs(RelCollationTraitDef.INSTANCE,
Review comment:
I agree that flexibility is nice but with the rules I would be more
conservative. I would prefer that our default instances all (excluding
converters and other special rules) match and transform from/to Logical
operators. I think that people who bother to build their own ruleset are
capable of instantiating the rules properly if needed. Those who don't are more
or less those using the Volcano planner as a black box with the default
ruleset. My opinion may be a bit biased since I am also in the same position as
@hsyuan.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
