[
https://issues.apache.org/jira/browse/CALCITE-5448?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17650853#comment-17650853
]
Benchao Li commented on CALCITE-5448:
-------------------------------------
bq. Perhaps another alternative to avoid adding a new getter
RexSimplify#getExecutor could be adding the RexExecutor executor as a parameter
in reduceExpressionsInternal
I agree with approach for this case.
And I also agree with [~julianhyde] that there should be only one place where
the {{Executor}} comes from, currently, it is the
{{RelOptPlanner#getExecutor}}.
Looking at the usages of {{RelOptPlanner#getExecutor}}, most of them are using
it like {{Util.first(cluster.getPlanner().getExecutor(), RexUtil.EXECUTOR)}}.
Except this case, there is [one more
place|https://github.com/apache/calcite/blob/013f034dee3e24083760b4695e2eacfbf592c2cb/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java#L3995-L3999]
which does not fallback to {{RexUtil.EXECUTOR}}, we should fix that too (maybe
in another separate issue).
In the long run, since we always needs an {{Executor}}, it would be better to
make the return of {{RelOptPlanner#getExecutor}} not null, and make the
fallback inside it.
> 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)
