You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by Vova Vysotskyi <vv...@gmail.com> on 2017/11/07 15:01:32 UTC
Make RelBuilder.filter() configurable
Hi all,
In CALCITE-1056 <https://issues.apache.org/jira/browse/CALCITE-1056> was
optimized planning by returning RelBuilder.empty() when the predicate in
the filter is always false.
Drill can dynamically describe columns at runtime, but some of them, or
their actual types may be unknown at planning time.
Drill is able to return column info for the queries that do not return any
results. But with the change which was made in CALCITE-1056
<https://issues.apache.org/jira/browse/CALCITE-1056>, it becomes impossible
since for the cases when a predicate is always false, RelBuilder.empty() is
returned.
Is it acceptable to make RelBuilder.filter() configurable to avoid
returning RelBuilder.empty()?
I propose to change condition on this line
<https://github.com/apache/calcite/blob/fb760a6f4e0cf8ccf92380dd676a72d00402d53e/core/src/main/java/org/apache/calcite/tools/RelBuilder.java#L921>
into *x.isAlwaysFalse() && simplify*.
Also, some other Hook may be created for this goal when we will decide to
use such approach.
--
Kind regards,
Volodymyr Vysotskyi
Re: Make RelBuilder.filter() configurable
Posted by Julian Hyde <jh...@apache.org>.
Feel free to add constructors to rules with a RelBuilderFactory
parameter anywhere that you need.
Obviously you should preserve public APIs but if the existing
constructor is private/protected and only used by a singleton feel
free to move the factory into the singleton's initialiizer.
Can you please log a JIRA case and let's continue discussion there.
Julian
On Tue, Nov 7, 2017 at 11:11 AM, Vova Vysotskyi <vv...@gmail.com> wrote:
> Some rules do not have constructors with RelBuilderFactory.
>
> Going through the rules that are used in Drill, I found these ones:
>
> - AggregateRemoveRule
> - ProjectRemoveRule (has private constructor)
> - SortRemoveRule (has private constructor)
> - ProjectWindowTransposeRule (has private constructor)
> - ExpandConversionRule (has private constructor)
> - ConverterRule
> - ExpandConversionRule (has private constructor)
> - JoinToMultiJoinRule
> - ProjectToWindowRule instance (has private constructor)
>
>
> Also, many of them have only private constructors.
>
> 2017-11-07 18:06 GMT+00:00 Julian Hyde <jh...@apache.org>:
>
>> Yes there are lots of rules that use one of the 2 "RelBuilderFactory
>> proto()" methods. I counted 40. But every single one of them is marked
>> "@Deprecated" and has an alternative. Does that tell you something? :)
>>
>> It's really worth spending the time to migrate all code that uses
>> deprecated parts of Calcite's API. Do it sooner rather than later.
>>
>> Julian
>>
>>
>> On Tue, Nov 7, 2017 at 7:40 AM, Vova Vysotskyi <vv...@gmail.com> wrote:
>> > Hi Julian,
>> >
>> > I tried to override this method, but I was not able to replace all usages
>> > of *RexBuilder* by my own implementation from Drill.
>> > Many instances of Calcite rules use static method *RelBuilderFactory
>> > proto()* which allows creating only *RelBuilder* instances or use
>> > *RelBuilderFactory* from *RelFactories *class.
>> >
>> > 2017-11-07 15:21 GMT+00:00 Julian Hyde <jh...@gmail.com>:
>> >
>> >> Read the javadoc of RelBuilder.empty(). Does that solve your problem?
>> >>
>> >> Julian
>> >>
>> >> > On Nov 7, 2017, at 7:01 AM, Vova Vysotskyi <vv...@gmail.com> wrote:
>> >> >
>> >> > Hi all,
>> >> >
>> >> > In CALCITE-1056 <https://issues.apache.org/jira/browse/CALCITE-1056>
>> was
>> >> > optimized planning by returning RelBuilder.empty() when the predicate
>> in
>> >> > the filter is always false.
>> >> >
>> >> > Drill can dynamically describe columns at runtime, but some of them,
>> or
>> >> > their actual types may be unknown at planning time.
>> >> >
>> >> > Drill is able to return column info for the queries that do not return
>> >> any
>> >> > results. But with the change which was made in CALCITE-1056
>> >> > <https://issues.apache.org/jira/browse/CALCITE-1056>, it becomes
>> >> impossible
>> >> > since for the cases when a predicate is always false,
>> RelBuilder.empty()
>> >> is
>> >> > returned.
>> >> >
>> >> > Is it acceptable to make RelBuilder.filter() configurable to avoid
>> >> > returning RelBuilder.empty()?
>> >> > I propose to change condition on this line
>> >> > <https://github.com/apache/calcite/blob/
>> fb760a6f4e0cf8ccf92380dd676a72
>> >> d00402d53e/core/src/main/java/org/apache/calcite/tools/
>> >> RelBuilder.java#L921>
>> >> > into *x.isAlwaysFalse() && simplify*.
>> >> > Also, some other Hook may be created for this goal when we will
>> decide to
>> >> > use such approach.
>> >> >
>> >> > --
>> >> > Kind regards,
>> >> > Volodymyr Vysotskyi
>> >>
>> >
>> >
>> >
>> > --
>> > Kind regards,
>> > Volodymyr Vysotskyi
>>
>
>
>
> --
> Kind regards,
> Volodymyr Vysotskyi
Re: Make RelBuilder.filter() configurable
Posted by Vova Vysotskyi <vv...@gmail.com>.
Some rules do not have constructors with RelBuilderFactory.
Going through the rules that are used in Drill, I found these ones:
- AggregateRemoveRule
- ProjectRemoveRule (has private constructor)
- SortRemoveRule (has private constructor)
- ProjectWindowTransposeRule (has private constructor)
- ExpandConversionRule (has private constructor)
- ConverterRule
- ExpandConversionRule (has private constructor)
- JoinToMultiJoinRule
- ProjectToWindowRule instance (has private constructor)
Also, many of them have only private constructors.
2017-11-07 18:06 GMT+00:00 Julian Hyde <jh...@apache.org>:
> Yes there are lots of rules that use one of the 2 "RelBuilderFactory
> proto()" methods. I counted 40. But every single one of them is marked
> "@Deprecated" and has an alternative. Does that tell you something? :)
>
> It's really worth spending the time to migrate all code that uses
> deprecated parts of Calcite's API. Do it sooner rather than later.
>
> Julian
>
>
> On Tue, Nov 7, 2017 at 7:40 AM, Vova Vysotskyi <vv...@gmail.com> wrote:
> > Hi Julian,
> >
> > I tried to override this method, but I was not able to replace all usages
> > of *RexBuilder* by my own implementation from Drill.
> > Many instances of Calcite rules use static method *RelBuilderFactory
> > proto()* which allows creating only *RelBuilder* instances or use
> > *RelBuilderFactory* from *RelFactories *class.
> >
> > 2017-11-07 15:21 GMT+00:00 Julian Hyde <jh...@gmail.com>:
> >
> >> Read the javadoc of RelBuilder.empty(). Does that solve your problem?
> >>
> >> Julian
> >>
> >> > On Nov 7, 2017, at 7:01 AM, Vova Vysotskyi <vv...@gmail.com> wrote:
> >> >
> >> > Hi all,
> >> >
> >> > In CALCITE-1056 <https://issues.apache.org/jira/browse/CALCITE-1056>
> was
> >> > optimized planning by returning RelBuilder.empty() when the predicate
> in
> >> > the filter is always false.
> >> >
> >> > Drill can dynamically describe columns at runtime, but some of them,
> or
> >> > their actual types may be unknown at planning time.
> >> >
> >> > Drill is able to return column info for the queries that do not return
> >> any
> >> > results. But with the change which was made in CALCITE-1056
> >> > <https://issues.apache.org/jira/browse/CALCITE-1056>, it becomes
> >> impossible
> >> > since for the cases when a predicate is always false,
> RelBuilder.empty()
> >> is
> >> > returned.
> >> >
> >> > Is it acceptable to make RelBuilder.filter() configurable to avoid
> >> > returning RelBuilder.empty()?
> >> > I propose to change condition on this line
> >> > <https://github.com/apache/calcite/blob/
> fb760a6f4e0cf8ccf92380dd676a72
> >> d00402d53e/core/src/main/java/org/apache/calcite/tools/
> >> RelBuilder.java#L921>
> >> > into *x.isAlwaysFalse() && simplify*.
> >> > Also, some other Hook may be created for this goal when we will
> decide to
> >> > use such approach.
> >> >
> >> > --
> >> > Kind regards,
> >> > Volodymyr Vysotskyi
> >>
> >
> >
> >
> > --
> > Kind regards,
> > Volodymyr Vysotskyi
>
--
Kind regards,
Volodymyr Vysotskyi
Re: Make RelBuilder.filter() configurable
Posted by Julian Hyde <jh...@apache.org>.
Yes there are lots of rules that use one of the 2 "RelBuilderFactory
proto()" methods. I counted 40. But every single one of them is marked
"@Deprecated" and has an alternative. Does that tell you something? :)
It's really worth spending the time to migrate all code that uses
deprecated parts of Calcite's API. Do it sooner rather than later.
Julian
On Tue, Nov 7, 2017 at 7:40 AM, Vova Vysotskyi <vv...@gmail.com> wrote:
> Hi Julian,
>
> I tried to override this method, but I was not able to replace all usages
> of *RexBuilder* by my own implementation from Drill.
> Many instances of Calcite rules use static method *RelBuilderFactory
> proto()* which allows creating only *RelBuilder* instances or use
> *RelBuilderFactory* from *RelFactories *class.
>
> 2017-11-07 15:21 GMT+00:00 Julian Hyde <jh...@gmail.com>:
>
>> Read the javadoc of RelBuilder.empty(). Does that solve your problem?
>>
>> Julian
>>
>> > On Nov 7, 2017, at 7:01 AM, Vova Vysotskyi <vv...@gmail.com> wrote:
>> >
>> > Hi all,
>> >
>> > In CALCITE-1056 <https://issues.apache.org/jira/browse/CALCITE-1056> was
>> > optimized planning by returning RelBuilder.empty() when the predicate in
>> > the filter is always false.
>> >
>> > Drill can dynamically describe columns at runtime, but some of them, or
>> > their actual types may be unknown at planning time.
>> >
>> > Drill is able to return column info for the queries that do not return
>> any
>> > results. But with the change which was made in CALCITE-1056
>> > <https://issues.apache.org/jira/browse/CALCITE-1056>, it becomes
>> impossible
>> > since for the cases when a predicate is always false, RelBuilder.empty()
>> is
>> > returned.
>> >
>> > Is it acceptable to make RelBuilder.filter() configurable to avoid
>> > returning RelBuilder.empty()?
>> > I propose to change condition on this line
>> > <https://github.com/apache/calcite/blob/fb760a6f4e0cf8ccf92380dd676a72
>> d00402d53e/core/src/main/java/org/apache/calcite/tools/
>> RelBuilder.java#L921>
>> > into *x.isAlwaysFalse() && simplify*.
>> > Also, some other Hook may be created for this goal when we will decide to
>> > use such approach.
>> >
>> > --
>> > Kind regards,
>> > Volodymyr Vysotskyi
>>
>
>
>
> --
> Kind regards,
> Volodymyr Vysotskyi
Re: Make RelBuilder.filter() configurable
Posted by Vova Vysotskyi <vv...@gmail.com>.
Hi Julian,
I tried to override this method, but I was not able to replace all usages
of *RexBuilder* by my own implementation from Drill.
Many instances of Calcite rules use static method *RelBuilderFactory
proto()* which allows creating only *RelBuilder* instances or use
*RelBuilderFactory* from *RelFactories *class.
2017-11-07 15:21 GMT+00:00 Julian Hyde <jh...@gmail.com>:
> Read the javadoc of RelBuilder.empty(). Does that solve your problem?
>
> Julian
>
> > On Nov 7, 2017, at 7:01 AM, Vova Vysotskyi <vv...@gmail.com> wrote:
> >
> > Hi all,
> >
> > In CALCITE-1056 <https://issues.apache.org/jira/browse/CALCITE-1056> was
> > optimized planning by returning RelBuilder.empty() when the predicate in
> > the filter is always false.
> >
> > Drill can dynamically describe columns at runtime, but some of them, or
> > their actual types may be unknown at planning time.
> >
> > Drill is able to return column info for the queries that do not return
> any
> > results. But with the change which was made in CALCITE-1056
> > <https://issues.apache.org/jira/browse/CALCITE-1056>, it becomes
> impossible
> > since for the cases when a predicate is always false, RelBuilder.empty()
> is
> > returned.
> >
> > Is it acceptable to make RelBuilder.filter() configurable to avoid
> > returning RelBuilder.empty()?
> > I propose to change condition on this line
> > <https://github.com/apache/calcite/blob/fb760a6f4e0cf8ccf92380dd676a72
> d00402d53e/core/src/main/java/org/apache/calcite/tools/
> RelBuilder.java#L921>
> > into *x.isAlwaysFalse() && simplify*.
> > Also, some other Hook may be created for this goal when we will decide to
> > use such approach.
> >
> > --
> > Kind regards,
> > Volodymyr Vysotskyi
>
--
Kind regards,
Volodymyr Vysotskyi
Re: Make RelBuilder.filter() configurable
Posted by Julian Hyde <jh...@gmail.com>.
Read the javadoc of RelBuilder.empty(). Does that solve your problem?
Julian
> On Nov 7, 2017, at 7:01 AM, Vova Vysotskyi <vv...@gmail.com> wrote:
>
> Hi all,
>
> In CALCITE-1056 <https://issues.apache.org/jira/browse/CALCITE-1056> was
> optimized planning by returning RelBuilder.empty() when the predicate in
> the filter is always false.
>
> Drill can dynamically describe columns at runtime, but some of them, or
> their actual types may be unknown at planning time.
>
> Drill is able to return column info for the queries that do not return any
> results. But with the change which was made in CALCITE-1056
> <https://issues.apache.org/jira/browse/CALCITE-1056>, it becomes impossible
> since for the cases when a predicate is always false, RelBuilder.empty() is
> returned.
>
> Is it acceptable to make RelBuilder.filter() configurable to avoid
> returning RelBuilder.empty()?
> I propose to change condition on this line
> <https://github.com/apache/calcite/blob/fb760a6f4e0cf8ccf92380dd676a72d00402d53e/core/src/main/java/org/apache/calcite/tools/RelBuilder.java#L921>
> into *x.isAlwaysFalse() && simplify*.
> Also, some other Hook may be created for this goal when we will decide to
> use such approach.
>
> --
> Kind regards,
> Volodymyr Vysotskyi