calcite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jinfeng Ni <jinfengn...@gmail.com>
Subject Re: Trait Propagation, requesting traits using RelOptRule.convert() & CALCITE-557
Date Thu, 05 Mar 2015 00:40:56 GMT
BTW: The branch I posted above is based on Jacques's CALCITE-606 branch [2]

[2]
https://github.com/jacques-n/incubator-calcite/tree/CALCITE-606

On Wed, Mar 4, 2015 at 4:37 PM, Jinfeng Ni <jinfengni99@gmail.com> wrote:

> I made some change to the rules defined in the testcase, and it seems to
> get the plan w/o thought in the testcase of withoutHack(). Certainly, the
> testcase withHack() will not work now, since I change the PhysProjRule.
> See [1].
>
> Here is the output of withoutHack() testcase:
>
> LOGICAL PLAN
> PhysAgg(group=[{0}], cnt=[COUNT($1)]): rowcount = 1.0, cumulative cost =
> {3.0 rows, 3.0 cpu, 3.0 io}, id = 80
>   PhysProj(s=[$0], i=[$1]): rowcount = 1.0, cumulative cost = {2.0 rows,
> 2.0 cpu, 2.0 io}, id = 79
>     PhysTable: rowcount = 1.0, cumulative cost = {1.0 rows, 1.0 cpu, 1.0
> io}, id = 32
>
> Here is main idea of the change.
>
> 1. Change PhysProjRule to a subclass of ConverterRule, and make it simply
> convert its child and its own from Logical ('NONE') to Physical convention.
>
> 2. Add a new PhysProjTraitPropUpRule.   This rule will match any *physical
> Project*, and will try to propagate the traits from the project's input's
> *best* RelNode.
>
> Seems to me both the new PhysProjRule and PhysProjTraitPropUpRule probably
> will not blow out the search space, since each rule fire will produce at
> most one new RelNode.
>
> The reason the original rule does not work, in my view, is.
>
>      The original PhysProjRule is fired, it *only matchs with
> LogicalProject*. However, the LogicalProject's child (SCAN) is also
> Logical.  Only when the SCAN is converted into physical, we'll add the
> RelCollation to the Physical Scan. Unfortunately, Physical Scan is not a
> child of LogicalProject. So, when a new Physical Scan node with the desired
> RelCollation is created, the original PhysProjRule will not be fired at
> all. Hence, no RelCollation trait propagation bottom-up.
>
> Does the above change make sense? Thanks!
>
> [1].
> https://github.com/jinfengni/incubator-optiq/tree/CALCITE-606-ALT
>
>
>
> On Wed, Mar 4, 2015 at 2:45 PM, Julian Hyde <julianhyde@gmail.com> wrote:
>
>> I’m working on this. Here are my two ideas:
>>
>> 1. Try adding SortRemoveRule.INSTANCE
>>
>> 2. Try calling the new “create” methods for each RelNode sub-class. For
>> example, LogicalProject.create:
>>
>> public static LogicalProject create(final RelNode input,
>>     final List<? extends RexNode> projects, RelDataType rowType) {
>>   final RelOptCluster cluster = input.getCluster();
>>   final RelTraitSet traitSet =
>>       cluster.traitSet().replace(Convention.NONE)
>>           .replaceIfs(
>>               RelCollationTraitDef.INSTANCE,
>>               new Supplier<List<RelCollation>>() {
>>                 public List<RelCollation> get() {
>>                   return RelMdCollation.project(input, projects);
>>                 }
>>               });
>>   return new LogicalProject(cluster, traitSet, input, projects, rowType);
>> }
>>
>> This method derives the collation trait, if enabled, before calling the
>> LogicalProject constructor. The caller does not need to derive the traits
>> (which is good, because the caller would likely get it wrong).
>>
>> The traits need to be defined before the super-class constructor is
>> called because the RelTraitSet is an immutable field of RelNode (it is not
>> final, but it should be, and a lot of code assumes that it is).
>>
>> Julian
>>
>>
>>
>> On Mar 1, 2015, at 8:55 AM, Jacques Nadeau <jacques@apache.org> wrote:
>>
>> > 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 <jacques@apache.org>
>> 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
>> >>
>>
>>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message