I open a PR - https://github.com/apache/calcite/pull/1543
> On Oct 28, 2019, at 3:00 PM, Xiening Dai <[email protected]> wrote: > > Thanks for you input. > > I think this doesn’t relates to the Volcano theory. I don’t try to separate > the logical transformation from physical implementation. What I propose is > not to fire the same transformation rule on physical nodes as those > transformation are already done on the logic nodes. > > I tested out my patch, and all UTs passed. I can open a PR for that. > > >> On Oct 28, 2019, at 2:18 PM, Stamatis Zampetakis <[email protected]> wrote: >> >> Hello, >> >> It is not surprising that by reducing the scope of the rules the >> planning times goes down since we are generating less alternatives. >> >> Mixing logical with physical optimizations is costly but it is one of the >> big benefits offered by the Volcano framework in contrast with the System-R >> style optimization where logical and physical transformations are >> completely separated. >> >> Indeed in some cases having the same rule fire for both matching logical >> and physical nodes does not make sense so we could apply some tunings. >> >> @Xiening: Have you tried running all tests with the modifications you >> mentioned? Aren't we missing any good plans? >> @Haisheng: What do you mean by saying that rules should match logical >> operators only? >> >> Best, >> Stamatis >> >> On Mon, Oct 28, 2019 at 8:54 PM Haisheng Yuan <[email protected]> >> wrote: >> >>> Agree, it is indeed redundant. We had a discussion about this in pull >>> request #1130 [1]. >>> >>> Many of these rules not only match physical operaors, but still generate >>> new logical operators. >>> >>> IMHO, rules should match logical operators only. >>> >>> [1] https://github.com/apache/calcite/pull/1130 >>> >>> On 2019/10/28 16:47:55, Xiening Dai wrote: >>>> Hi all, >>>> >>>> While I was looking at CALCITE-2970, I noticed that some of the rules >>> are fired for both logical and physical nodes. For example, >>> ProjectMergeRule matches Project.class, so it’s fired for LogicalProject. >>> But then after LogicalProject is converted into EnummerableProject, the >>> same rule is fired again for the physical rels. Same for >>> EnumerableLimitRule, SortRemoveConstantKeysRule, etc. >>>> >>>> This seems to be unnecessary. When ProjectMerge is applied to >>> LogicalProject nodes, we already generate all possible alternatives with >>> merged projects. We just need to convert the LogicalProject into >>> EnumerableProject. There’s no need to merge EnumerableProject again. >>>> >>>> If I update those rules to only match logical nodes, the planning time >>> of the case in CALCITE-2970 is reduced ~30%. >>>> >>>> Any thoughts? >>> >>> >
