calcite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Julian Hyde <julianh...@gmail.com>
Subject Re: Trait Propagation, requesting traits using RelOptRule.convert() & CALCITE-557
Date Wed, 04 Mar 2015 22:45:24 GMT
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
View raw message