[
https://issues.apache.org/jira/browse/CALCITE-5448?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17649869#comment-17649869
]
Ruben Q L edited comment on CALCITE-5448 at 12/20/22 4:16 PM:
--------------------------------------------------------------
[~julianhyde], I agree that is the general case. But in the particular case of
{{ReduceExpressionsRule}}, if we look at the code that calls
{{reduceExpressionsInternal}}, the {{RexSimply}} is built using either the
executor from the planner, or as a fallback the "default" executor in RexUtil
[1]:
{code}
final RexExecutor executor = Util.first(cluster.getPlanner().getExecutor(),
RexUtil.EXECUTOR);
final RexSimplify simplify = new RexSimplify(rexBuilder, predicates,
executor);
final RexUnknownAs unknownAs = RexUnknownAs.falseIf(unknownAsFalse);
final boolean reduced = reduceExpressionsInternal(rel, simplify, unknownAs,
expList, predicates, treatDynamicCallsAsConstant);
{code}
IMO it seems a bit strange to use the {{RexUtil.EXECUTOR}} "fallback" in there,
and not insider {{reduceExpressionsInternal}}.
Perhaps another alternative to avoid adding a new getter
{{RexSimplify#getExecutor}} could be adding the {{RexExecutor executor}} as a
parameter in {{reduceExpressionsInternal}}
[1]
https://github.com/apache/calcite/blob/013f034dee3e24083760b4695e2eacfbf592c2cb/core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java#L702
was (Author: rubenql):
[~julianhyde], I agree that is the general case. But in the particular case of
{{ReduceExpressionsRule}}, if we look at the code that calls
{{reduceExpressionsInternal}}, the {{RexSimply}} is built using either the
executor from the planner, or as a fallback the "default" executor in RexUtil
[1]:
{code}
final RexExecutor executor = Util.first(cluster.getPlanner().getExecutor(),
RexUtil.EXECUTOR);
final RexSimplify simplify = new RexSimplify(rexBuilder, predicates,
executor);
// Simplify predicates in place
final RexUnknownAs unknownAs = RexUnknownAs.falseIf(unknownAsFalse);
final boolean reduced = reduceExpressionsInternal(rel, simplify, unknownAs,
expList, predicates, treatDynamicCallsAsConstant);
{code}
IMO it seems a bit strange to use the {{RexUtil.EXECUTOR}} "fallback" in there,
and not insider {{reduceExpressionsInternal}}.
Perhaps another alternative to avoid adding a new getter
{{RexSimplify#getExecutor}} could be adding the {{RexExecutor executor}} as a
parameter in {{reduceExpressionsInternal}}
[1]
https://github.com/apache/calcite/blob/013f034dee3e24083760b4695e2eacfbf592c2cb/core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java#L702
> ReduceExpressionsRule does not always constant fold expressions
> ----------------------------------------------------------------
>
> Key: CALCITE-5448
> URL: https://issues.apache.org/jira/browse/CALCITE-5448
> Project: Calcite
> Issue Type: Bug
> Components: core
> Affects Versions: 1.32.0
> Reporter: Mihai Budiu
> Priority: Minor
>
> I have manually built a HepPlanner to optimize the SQL queries, and I
> discovered that the rule ReduceExpressionsRule does not really do anything in
> my setup.
> I am looking at method ReduceExpressionsRule.reduceExpressionsInternal.
> There is this piece of code:
> {code}
> RexExecutor executor = rel.getCluster().getPlanner().getExecutor();
> if (executor == null) {
> // Cannot reduce expressions: caller has not set an executor in their
> // environment. Caller should execute something like the following
> before
> // invoking the planner:
> //
> // final RexExecutorImpl executor =
> // new RexExecutorImpl(Schemas.createDataContext(null));
> // rootRel.getCluster().getPlanner().setExecutor(executor);
> return changed;
> }
> {code}
>
> However, the caller of this function, the method reduceExpressions, has
> carefully inserted an executor in the RexSimplify class in case the cluster
> has no executor. Shouldn't that executor be used instead of trying the
> missing one? (It is currently private in the RexSimplify class.)
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
