julianhyde 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_r272735658
##########
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.
It would be nice if the default instance of these rules matched Filter,
Project and Aggregate of any convention and automatically transformed to
Filter, Project and Aggregate of the same convention. I know that's asking a
lot. But it would be a user-friendly default for people who don't want to go to
the effort of building rule sets and multi-phase planning.
People who are building complex planners would, I'm sure, create instances
of those rules that match only one convention.
I'd also love to deprecate the `RelNode.copy` method and all of its variants
in sub-classes. That is the reason why these rules behave inconsistently
between single-convention and multi-convention mode.
Could we add a method `RelBuilder RelBuilder.withConvention(Convention)`?
Thus rather than
```
filter.copy(traitSet, input, newCondition)
```
you'd write
```
relBuilder.withConvention(filter.getConvention)
.push(input)
.filter(newCondition)
.build()
```
----------------------------------------------------------------
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
