So many threads. Vladimir asked whether I thought it was OK that a Logical RelNode should have an input that is not logical. Yes, I do. LogicalFilter(JdbcTableScan) for example (and, more generally, our use of toRel to create a table that is not logical, but has a particular convention).
Allowing heterogenous graphs is fundamental to how Calcite operates. It’s POSSIBLE that we could make it operate differently, but why? It seems to me that Vladimir is trying to introduce a particular kind of order because it seems “tidy”, not because anything is fundamentally broken. One of the good things about Calcite is that it allows considerable freedom to people writing rules. If a rule does the wrong thing, don’t blame the engine, fix the rule. It’s worth considering a scheme where rules opt in to a particular categorization. We already have converters, that declare their input and output conventions. We could have “single convention” rules, that consume and produce RelNodes of a given convention; and “convention preserving” rules, that consume and produce RelNodes of multiple conventions, but they have to be the same convention for a given rule firing. Also, if a particular RelNode requires its input to be a particular trait (for example, EnumerableFilter requires its input to be Enumerable) then its constructor should assert. That is much simpler, and more direct, than requiring support from rules and engine. That’s the embodiment of the principle of least astonishment. Julian > On Feb 2, 2019, at 2:26 PM, Vladimir Sitnikov <[email protected]> > wrote: > >> LogicalFilter(EnumerableFilter(LogicalProject(...))) kind of nodes. > > RelCollationTraitDef is an interesting case: it creates > LogicalSort.create then it somehow converts it to proper subtype. > > During that conversion LogicalSort consumes DrillPhysical subset: > java.lang.AssertionError: Input #0 > rel#865:Subset#59.PHYSICAL.HASH_DISTRIBUTED([[$2]]).[] of > LogicalSort#883 does not satisfy required trait NONE > at org.apache.calcite.util.Litmus$1.fail(Litmus.java:31) > ~[calcite-core-1.18.0-drill.jar:1.18.0-drill] > at > org.apache.calcite.rel.AbstractRelNode.inputsSatisfy(AbstractRelNode.java:191) > ~[calcite-core-1.18.0-drill.jar:1.18.0-drill] > at org.apache.calcite.rel.logical.LogicalSort.<init>(LogicalSort.java:40) > ~[calcite-core-1.18.0-drill.jar:1.18.0-drill] > at org.apache.calcite.rel.logical.LogicalSort.create(LogicalSort.java:65) > ~[calcite-core-1.18.0-drill.jar:1.18.0-drill] > at > org.apache.calcite.rel.RelCollationTraitDef.convert(RelCollationTraitDef.java:75) > ~[calcite-core-1.18.0-drill.jar:1.18.0-drill] > at > org.apache.calcite.rel.RelCollationTraitDef.convert(RelCollationTraitDef.java:39) > ~[calcite-core-1.18.0-drill.jar:1.18.0-drill] > at > org.apache.calcite.plan.volcano.VolcanoPlanner.changeTraitsUsingConverters(VolcanoPlanner.java:1031) > ~[calcite-core-1.18.0-drill.jar:1.18.0-drill] > at > org.apache.calcite.plan.volcano.VolcanoPlanner.changeTraitsUsingConverters(VolcanoPlanner.java:1122) > ~[calcite-core-1.18.0-drill.jar:1.18.0-drill] > at > org.apache.calcite.plan.volcano.AbstractConverter$ExpandConversionRule.onMatch(AbstractConverter.java:122) > ~[calcite-core-1.18.0-drill.jar:1.18.0-drill] > at > org.apache.calcite.plan.volcano.VolcanoRuleCall.onMatch(VolcanoRuleCall.java:214) > ~[calcite-core-1.18.0-drill.jar:1.18.0-drill] > at > org.apache.calcite.plan.volcano.VolcanoPlanner.findBestExp(VolcanoPlanner.java:642) > ~[calcite-core-1.18.0-drill.jar:1.18.0-drill] > > Vladimir
