Interestingly, I was looking at this same piece of code not so long ago and I agree it is a bit confusing.
Looking around the places that we obtain a RexExecutor, most often (always?) we observe the following pattern: RexExecutor executor = Util.first(query.getCluster().getPlanner().getExecutor(), RexUtil.EXECUTOR); I think it is always useful to have an executor in the planner thus I am tempted to change the API of RelOptPlanner#getExecutor to always return an (default) executor if an explicit one is not set. The API of RexExecutor says the following "If an expression cannot be reduced, writes the original expression..." so we don't break anything by providing a default one. What do you think? Best, Stamatis On Mon, Mar 16, 2020 at 11:11 AM Danny Chan <[email protected]> wrote: > Thanks, the code is a little mess, here is how I understand it: > > The executor from `final RexExecutor executor > = Util.first(cluster.getPlanner().getExecutor(), RexUtil.EXECUTOR)` is > mainly used to construct the RexSimplify, in the RexSimplify, the > expression that we evaluate is what we can make sure RexUtil.EXECUTOR can > resolve(if you check the code, it only reduce the literals). > > But the expressions in the ReduceExpressionsRule may be more complex, > somehow we must relay on the engine to plugin their RexExecutor to make a > constant reduction(some engine use code generation, some use Java > reflection). > > So, in total, the executor in RexSimplify has a fallback is because it’s > expression to reduce is simple enough. > > Best, > Danny Chan > 在 2020年3月16日 +0800 PM3:57，JiaTao Tao <[email protected]>，写道： > > In method reduceExpressionsInternal, we get RexExecutor from cluster, it > can be null: > > <> > > > > But in the outside(reduceExpressions), `final RexExecutor executor = > Util.first(cluster.getPlanner().getExecutor(), RexUtil.EXECUTOR)`, it can't > be null. > > > > And reduceExpressions is the only caller of reduceExpressionsInternal, > so I think this is an inconsistent behavior. > > > > IMHO, we should create RexUtil.EXECUTOR if it is null > in reduceExpressionsInternal, or just get RexExecutor from RexSimplify. > > <> > > > > Regards! > > Aron Tao >
