danny0405 commented on a change in pull request #1586: [CALCITE-3491]
VolcanoPlanner.completeConversion() is bypassed by "if…
URL: https://github.com/apache/calcite/pull/1586#discussion_r346616014
##########
File path:
core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java
##########
@@ -1081,60 +1072,6 @@ private RelNode changeTraitsUsingConverters(
return converted;
}
- /**
- * Converts traits using well-founded induction. We don't require that
- * each conversion preserves all traits that have previously been converted,
- * but if it changes "locked in" traits we'll try some other conversion.
- *
- * @param rel Relational expression
- * @param allowInfiniteCostConverters Whether to allow infinite converters
- * @param toTraits Target trait set
- * @param usedTraits Traits that have been locked in
- * @return Converted relational expression
- */
- private RelNode completeConversion(
- RelNode rel,
- boolean allowInfiniteCostConverters,
- RelTraitSet toTraits,
- Expressions.FluentList<RelTraitDef> usedTraits) {
- if (true) {
- return rel;
Review comment:
Please hold on for the fix, i'm curious if this logic is left there
intentionally. @julianhyde Can you please take a look at this code ? There are
many places in Calcite with the special "useless" `if(true)` logic, for me, it
makes the logic more clear because it illustrates how the logic evolves.
----------------------------------------------------------------
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
