You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by Ruben Q L <ru...@gmail.com> on 2022/02/18 15:21:33 UTC

[HELP][DISCUSS] ReduceExpressionsRule configurability / extensibility

Hello community,

I would need some advice for a very specific problem.
I find myself in the following situation: I have a plan with a bottom
filter (an equality) and a top filter (a user defined function MY_FUNC):
...
  LogicalFilter(condition: MY_FUNC($0, 42))
    LogicalFilter(condition: $0=1)
      ...

When ReduceExpressionsRule.FilterReduceExpressionsRule gets applied on the
top filter, it pulls up the predicates from the bottom, detects that $0 is
equal to 1, so it replaces it leaving:
  LogicalFilter(condition: MY_FUNC(1, 42))
    LogicalFilter(condition: $0=1)

The relevant code seems to be
ReduceExpressionsRule.ReducibleExprLocator#visitCall which considers that
all calls are a priori reducible:
    @Override public Void visitCall(RexCall call) {
      // assume REDUCIBLE_CONSTANT until proven otherwise
      analyzeCall(call, Constancy.REDUCIBLE_CONSTANT);
      return null;
    }

However, due to some unrelated circumstances, this reduction is incorrect
for my particular UDF, so I do not want it to be converted into MY_FUNC(1,
42), I'd need it to remain as MY_FUNC($0, 42) (i.e. neither the call
itself, nor its parameters can be reduced); the rest of the logic inside
FilterReduceExpressionsRule is perfectly fine for me. So it seems I'm
looking for something like:
    @Override public Void visitCall(RexCall call) {
     if (call.op.equals(MY_FUNC) {
       return pushVariable();
      }
      analyzeCall(call, Constancy.REDUCIBLE_CONSTANT);
      return null;
    }

Question 1: Is there a way to achieve this result (i.e. let it know to
ReduceExpressionsRule that a certain operator must not be reduced) via rule
configuration or in the UDF operator's definition?

So far, I have not found a positive answer to this question, so my next
thought was "ok, I'll define my own MyFilterReduceExpressionsRule which
extends FilterReduceExpressionsRule and will adjust the few parts of the
code where I need some special treatment, i.e. ReducibleExprLocator''.
Except that, in practice I cannot simply do that, I am forced to copy-paste
most of the original rule code (and then modify a few lines) because of the
following reasons:
- ReduceExpressionsRule subclasses (e.g. FilterReduceExpressionsRule) even
if they are protected, they use some auxiliary methods that are static, so
they cannot be overridden (e.g. FilterReduceExpressionsRule#onMatch calls
the static reduceExpressions that calls the static
reduceExpressionsInternal that calls the static findReducibleExps that
creates the ReducibleExprLocator).
- ReduceExpressionsRule uses some auxiliary static classes (e.g.
ReducibleExprLocator) which are protected (good) but have a package-private
constructor (bad) so in practice they cannot be extended (I cannot create
"MyReducibleExprLocator extends ReducibleExprLocator" to deal with my
special UDF).

Question 2: if the answer to the first question is "no", should we improve
ReduceExpressionsRule to make it more easily adaptable (for cases like my
example)? Maybe converting the static methods into non-static; and
declaring the static classes' constructors protected (so that anything can
be overridden by downstream rule subclasses if required)? Or maybe we could
provide more (optional) configuration capabilities in
ReduceExpressionsRule.Config to achieve this?

Best regards,
Ruben

Re: [HELP][DISCUSS] ReduceExpressionsRule configurability / extensibility

Posted by Ruben Q L <ru...@gmail.com>.
Thanks Stamatis, I will take a look at those links.


On Fri, Feb 18, 2022 at 9:41 PM Stamatis Zampetakis <za...@gmail.com>
wrote:

> Hi Ruben,
>
> There was a recent request about preventing simplifications of certain
> operators [1] that does make sense in certain use cases. Apart from changes
> in RexSimplify this request would most likely need some kind of changes in
> various reduce expressions rules like the ones you seem to need as well.
>
> Adding appropriate configuration to the respective rule to avoid reducing
> certain expressions seems reasonable to me. Note that some other reduction
> rules, such as AggregateReduceFunctionsRule [2], already expose similar
> configurations.
>
> Best,
> Stamatis
>
> [1] https://lists.apache.org/thread/cyj792yqfc8byfkxcw2jv07c9tfs0np9
> [2]
>
> https://github.com/apache/calcite/blob/9c4f3bb540dd67a0ffefc09f4ebd98d2be65bb14/core/src/main/java/org/apache/calcite/rel/rules/AggregateReduceFunctionsRule.java#L871
>
> On Fri, Feb 18, 2022 at 4:23 PM Ruben Q L <ru...@gmail.com> wrote:
>
> > Hello community,
> >
> > I would need some advice for a very specific problem.
> > I find myself in the following situation: I have a plan with a bottom
> > filter (an equality) and a top filter (a user defined function MY_FUNC):
> > ...
> >   LogicalFilter(condition: MY_FUNC($0, 42))
> >     LogicalFilter(condition: $0=1)
> >       ...
> >
> > When ReduceExpressionsRule.FilterReduceExpressionsRule gets applied on
> the
> > top filter, it pulls up the predicates from the bottom, detects that $0
> is
> > equal to 1, so it replaces it leaving:
> >   LogicalFilter(condition: MY_FUNC(1, 42))
> >     LogicalFilter(condition: $0=1)
> >
> > The relevant code seems to be
> > ReduceExpressionsRule.ReducibleExprLocator#visitCall which considers that
> > all calls are a priori reducible:
> >     @Override public Void visitCall(RexCall call) {
> >       // assume REDUCIBLE_CONSTANT until proven otherwise
> >       analyzeCall(call, Constancy.REDUCIBLE_CONSTANT);
> >       return null;
> >     }
> >
> > However, due to some unrelated circumstances, this reduction is incorrect
> > for my particular UDF, so I do not want it to be converted into
> MY_FUNC(1,
> > 42), I'd need it to remain as MY_FUNC($0, 42) (i.e. neither the call
> > itself, nor its parameters can be reduced); the rest of the logic inside
> > FilterReduceExpressionsRule is perfectly fine for me. So it seems I'm
> > looking for something like:
> >     @Override public Void visitCall(RexCall call) {
> >      if (call.op.equals(MY_FUNC) {
> >        return pushVariable();
> >       }
> >       analyzeCall(call, Constancy.REDUCIBLE_CONSTANT);
> >       return null;
> >     }
> >
> > Question 1: Is there a way to achieve this result (i.e. let it know to
> > ReduceExpressionsRule that a certain operator must not be reduced) via
> rule
> > configuration or in the UDF operator's definition?
> >
> > So far, I have not found a positive answer to this question, so my next
> > thought was "ok, I'll define my own MyFilterReduceExpressionsRule which
> > extends FilterReduceExpressionsRule and will adjust the few parts of the
> > code where I need some special treatment, i.e. ReducibleExprLocator''.
> > Except that, in practice I cannot simply do that, I am forced to
> copy-paste
> > most of the original rule code (and then modify a few lines) because of
> the
> > following reasons:
> > - ReduceExpressionsRule subclasses (e.g. FilterReduceExpressionsRule)
> even
> > if they are protected, they use some auxiliary methods that are static,
> so
> > they cannot be overridden (e.g. FilterReduceExpressionsRule#onMatch calls
> > the static reduceExpressions that calls the static
> > reduceExpressionsInternal that calls the static findReducibleExps that
> > creates the ReducibleExprLocator).
> > - ReduceExpressionsRule uses some auxiliary static classes (e.g.
> > ReducibleExprLocator) which are protected (good) but have a
> package-private
> > constructor (bad) so in practice they cannot be extended (I cannot create
> > "MyReducibleExprLocator extends ReducibleExprLocator" to deal with my
> > special UDF).
> >
> > Question 2: if the answer to the first question is "no", should we
> improve
> > ReduceExpressionsRule to make it more easily adaptable (for cases like my
> > example)? Maybe converting the static methods into non-static; and
> > declaring the static classes' constructors protected (so that anything
> can
> > be overridden by downstream rule subclasses if required)? Or maybe we
> could
> > provide more (optional) configuration capabilities in
> > ReduceExpressionsRule.Config to achieve this?
> >
> > Best regards,
> > Ruben
> >
>

Re: [HELP][DISCUSS] ReduceExpressionsRule configurability / extensibility

Posted by Stamatis Zampetakis <za...@gmail.com>.
Hi Ruben,

There was a recent request about preventing simplifications of certain
operators [1] that does make sense in certain use cases. Apart from changes
in RexSimplify this request would most likely need some kind of changes in
various reduce expressions rules like the ones you seem to need as well.

Adding appropriate configuration to the respective rule to avoid reducing
certain expressions seems reasonable to me. Note that some other reduction
rules, such as AggregateReduceFunctionsRule [2], already expose similar
configurations.

Best,
Stamatis

[1] https://lists.apache.org/thread/cyj792yqfc8byfkxcw2jv07c9tfs0np9
[2]
https://github.com/apache/calcite/blob/9c4f3bb540dd67a0ffefc09f4ebd98d2be65bb14/core/src/main/java/org/apache/calcite/rel/rules/AggregateReduceFunctionsRule.java#L871

On Fri, Feb 18, 2022 at 4:23 PM Ruben Q L <ru...@gmail.com> wrote:

> Hello community,
>
> I would need some advice for a very specific problem.
> I find myself in the following situation: I have a plan with a bottom
> filter (an equality) and a top filter (a user defined function MY_FUNC):
> ...
>   LogicalFilter(condition: MY_FUNC($0, 42))
>     LogicalFilter(condition: $0=1)
>       ...
>
> When ReduceExpressionsRule.FilterReduceExpressionsRule gets applied on the
> top filter, it pulls up the predicates from the bottom, detects that $0 is
> equal to 1, so it replaces it leaving:
>   LogicalFilter(condition: MY_FUNC(1, 42))
>     LogicalFilter(condition: $0=1)
>
> The relevant code seems to be
> ReduceExpressionsRule.ReducibleExprLocator#visitCall which considers that
> all calls are a priori reducible:
>     @Override public Void visitCall(RexCall call) {
>       // assume REDUCIBLE_CONSTANT until proven otherwise
>       analyzeCall(call, Constancy.REDUCIBLE_CONSTANT);
>       return null;
>     }
>
> However, due to some unrelated circumstances, this reduction is incorrect
> for my particular UDF, so I do not want it to be converted into MY_FUNC(1,
> 42), I'd need it to remain as MY_FUNC($0, 42) (i.e. neither the call
> itself, nor its parameters can be reduced); the rest of the logic inside
> FilterReduceExpressionsRule is perfectly fine for me. So it seems I'm
> looking for something like:
>     @Override public Void visitCall(RexCall call) {
>      if (call.op.equals(MY_FUNC) {
>        return pushVariable();
>       }
>       analyzeCall(call, Constancy.REDUCIBLE_CONSTANT);
>       return null;
>     }
>
> Question 1: Is there a way to achieve this result (i.e. let it know to
> ReduceExpressionsRule that a certain operator must not be reduced) via rule
> configuration or in the UDF operator's definition?
>
> So far, I have not found a positive answer to this question, so my next
> thought was "ok, I'll define my own MyFilterReduceExpressionsRule which
> extends FilterReduceExpressionsRule and will adjust the few parts of the
> code where I need some special treatment, i.e. ReducibleExprLocator''.
> Except that, in practice I cannot simply do that, I am forced to copy-paste
> most of the original rule code (and then modify a few lines) because of the
> following reasons:
> - ReduceExpressionsRule subclasses (e.g. FilterReduceExpressionsRule) even
> if they are protected, they use some auxiliary methods that are static, so
> they cannot be overridden (e.g. FilterReduceExpressionsRule#onMatch calls
> the static reduceExpressions that calls the static
> reduceExpressionsInternal that calls the static findReducibleExps that
> creates the ReducibleExprLocator).
> - ReduceExpressionsRule uses some auxiliary static classes (e.g.
> ReducibleExprLocator) which are protected (good) but have a package-private
> constructor (bad) so in practice they cannot be extended (I cannot create
> "MyReducibleExprLocator extends ReducibleExprLocator" to deal with my
> special UDF).
>
> Question 2: if the answer to the first question is "no", should we improve
> ReduceExpressionsRule to make it more easily adaptable (for cases like my
> example)? Maybe converting the static methods into non-static; and
> declaring the static classes' constructors protected (so that anything can
> be overridden by downstream rule subclasses if required)? Or maybe we could
> provide more (optional) configuration capabilities in
> ReduceExpressionsRule.Config to achieve this?
>
> Best regards,
> Ruben
>