calcite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Julian Hyde <jh...@apache.org>
Subject Re: Transitioning rules from ProjectFactory etc. to RelBuilder
Date Wed, 23 Sep 2015 20:28:54 GMT
Great question.

For that case, I think you should carry on making a separate rule
instance with operands that contain classes. RelBuilder is a good
single point to put a lot of the configuration, but it is taking it
too far to put the target classes in there. I don't want its role as
"point for configuration for rules" to overwhelm its main role "an
easy way to build relational algebra expressions".

And besides, operands are powerful and descriptive, and because the
engine understands them it can short-cut which rules are fired.

For what it's worth, there's another piece of configuration that I
considered putting into RelBuilder but decided not to. That is,
whether a rule instance should call RelNode.copy to automatically
create RelNodes the same type as its inputs. See I introduced
copyFilter and copyProject arguments in FilterProjectTransposeRule:

https://github.com/julianhyde/incubator-calcite/blob/8099cf3151a462b06a4cbb1c0cad8f7e90881ca5/core/src/main/java/org/apache/calcite/rel/rules/FilterProjectTransposeRule.java#L67

rather than introduce new methods RelBuilder.shouldCopyProject() and
.shouldCopyFilter().

And by the way I also considered adding methods to RelBuilderFactory
(formerly known as ProtoRelBuilder). Adding methods there would not
"pollute" RelBuilder as badly.

Julian


On Wed, Sep 23, 2015 at 12:18 PM, Jinfeng Ni <jinfengni99@gmail.com> wrote:
> +1.
>
> I have one question.  RelBuilder seems to be used in onMatch() method,
> when a particular type of RelNode has to be created.  What about
> RelOptRuleOperand in the constructor of rule? Let's say if we want to
> rule to match only DrillProject, or HiveFilter. The current way seems to
> be that we extend the rule by providing a different RelOptRuleOperand.
>
> Can RelBuilder also have getFilterClass(), getProjectClass() etc, which
> would be used to specify the rule patten matching?
>
>
>
> On Tue, Sep 22, 2015 at 6:41 PM, Jacques Nadeau <jacques@apache.org> wrote:
>> Agree on the utility of this. +1.
>>
>> On Tue, Sep 22, 2015 at 6:22 PM, John Pullokkaran <
>> jpullokkaran@hortonworks.com> wrote:
>>
>>> This is useful, though Hive will have to do some refactoring.
>>>
>>> In past Hive had to copy bunch of rules to work around these issues.
>>> Rules & relnodes for org.apache.calcite.rel.logical.* is a good example of
>>> this.
>>>
>>>
>>> John
>>>
>>> On 9/22/15, 5:14 PM, "Julian Hyde" <jhyde@apache.org> wrote:
>>>
>>> >I have started work on
>>> >https://issues.apache.org/jira/browse/CALCITE-828 and my
>>> >work-in-progress is in
>>> >https://github.com/julianhyde/incubator-calcite/commits/828-rule-builder.
>>> >
>>> >This will be a big change to teams such as Hive and Drill. Now would
>>> >be a good time to chime in.
>>> >
>>> >The plan is that every rule instance has a ProtoRelBuilder field from
>>> >which a RelBuilder can be created to be used in an onMatch method. The
>>> >RelBuilder contains factories for every kind of RelNode, so we won't
>>> >have to keep plumbing new factories in to existing rules.
>>> >
>>> >There will be other advantages. RelBuilder is an easier and more
>>> >concise way of creating relational expressions. It does some useful
>>> >stuff for free, like flattening ANDs and eliminating identity
>>> >projections. I'm seeing the volume of code decrease and a few minor
>>> >plan improvements.
>>> >
>>> >I haven't yet found a good way to deal with the pattern where if, say,
>>> >ProjectFactory is null the rule is to use Project.copy to create a
>>> >Project of the same type.
>>> >
>>> >I'm keeping & deprecating the old rule constructors that take a
>>> >variety of XxxFactory arguments.
>>> >
>>> >As always, we try to keep API compatibility and mark the old API
>>> >
>>> >  @Deprecated // to be removed before 2.0
>>> >
>>> >but the deprecated APIs are no longer tested and are likely to be
>>> >flaky. We strongly suggest that you fix calls to deprecated APIs as
>>> >soon as you upgrade to a more recent version of Calcite.
>>> >
>>> >Julian
>>> >
>>>
>>>

Mime
View raw message