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_r272547670
##########
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 don't like the inconsistency either but I am not sure that the rules you
mentioned really do what they are supposed to. Matching an EnumerableFilter and
creating a LogicalFilter does not seem like the expected behavior. This
behavior may lead to problems like the one mentioned in CALCITE-2223. @vlsi
started a discussion in the dev list (subject "Principle of least astonishment
when dealing with Rules and RelBuidlerFactory" but unfortunately we didn't
conclude to how we should deal with such rules. I think we could remove the
`copyFilter`, `copyProject` but then the default rule (`INSTANCE`) should match
only `LogicalFilter.class` and `LogicalProject.class`.
----------------------------------------------------------------
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
