I've created CALCITE-606 to track this and committed some changes to [1]. Julian, I'd love for you to look at my updated test. I have two tests: one with a propagation hack and one without. I'd love your thoughts on how to get the one without to work.
thanks, Jacques [1] https://github.com/jacques-n/incubator-calcite/tree/CALCITE-606 On Sat, Feb 28, 2015 at 8:49 PM, Jacques Nadeau <[email protected]> wrote: > Hey Guys, > > I was tired of a hack we use in Drill to make trait propagation work as it > blows out the plan space. As such, I was building a simple example to > discuss here. Of course, I ran across additional difficulties that I would > like to discuss. > > I happen to build my initial example on an old copy of Calcite and got to > the point where I was illustrating the fact that we weren't doing trait > propagation. However, rather than post that as an example, I wanted to > move to the latest Calcite so we could discuss in that context. > > After updating all the code I've found a couple places where things have > changed and I'm not even sure how to implement my original pattern. The > biggest issue that changed things looks like CALCITE-557 [1] since it > doesn't guarantee a converter is created to ensure a non-root trait > conversion at planning time. I imagine I'm missing something in the new > paradigm but I'm not sure how to resolve given these changes. > > The simple example is: > > We have a logical plan that is Agg > Project > EnumTableAccess. > > We then convert to a physical plan PhysAgg > PhysSort > PhysProj > PhysScan > > The PhysAgg requires a collation trait since we'll imagine it is a > streaming aggregate. To illustrate the trait propagation problem, I've > defined PhysScan to expose the required collation already. The goal is to > get the PhysProj to propagate the collation trait so the PhysAgg doesn't > require insertion of a PhysSort. > > On the old Optiq branch (~4 months back), I get the following plans using > [2]: > --LOGICAL PLAN-- > AggregateRel(group=[{0}], cnt=[COUNT($1)]) > ProjectRel(s=[$0]) > EnumerableTableAccessRel(table=[[t1]]) > > --PHYSICAL PLAN-- > PhysAgg(group=[{0}], cnt=[COUNT($1)]) > PhysSort(sort0=[$0], dir0=[ASC-nulls-first]) > PhysProj(s=[$0]) > PhysTable // dir0=[ASC-nulls-first] > > > On the new Branch using [3], I can't actually get this to plan because of > the removal in [1]. > > Is there a test case for trait propagation or simply a RelOptRule imposing > an additional trait that works after 557 was merged? > > Thanks, > Jacques > > > > [1] > https://github.com/apache/incubator-calcite/commit/506c1ab46b6b3e0279c799acc4e4412d542e8b6d#diff-8 > [2] https://gist.github.com/jacques-n/2f004311bd0ead714ca4 > [3] https://gist.github.com/jacques-n/01ed0203039374688710 >
