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:37:12 GMT
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