Re: [DISCUSS] get RexExecutor from RexSimplify in method reduceExpressionsInternal

2020-03-24 Thread Chunwei Lei
IMHO, A disadvantage of supplying a default RexExecutor is that we cannot make sure that the reduced result is the same as the result of the execution engine, especially when there is some customized implementation. Best, Chunwei On Fri, Mar 20, 2020 at 11:16 AM Danny Chan wrote: > This is a

Re: [DISCUSS] get RexExecutor from RexSimplify in method reduceExpressionsInternal

2020-03-19 Thread Danny Chan
This is a preference, I would prefer the default value to not throw exceptions. Best, Danny Chan 在 2020年3月18日 +0800 PM3:53,Stamatis Zampetakis ,写道: > If a Janino exception comes up then it is a bug that we have to fix since > it violates the contract of the interface. > > From my point of view the

Re: [DISCUSS] get RexExecutor from RexSimplify in method reduceExpressionsInternal

2020-03-18 Thread Stamatis Zampetakis
If a Janino exception comes up then it is a bug that we have to fix since it violates the contract of the interface. >From my point of view the modification is meaningful for two reasons: * improves code readability; * avoids confusing behavior where the rules for performing constant reduction are

Re: [DISCUSS] get RexExecutor from RexSimplify in method reduceExpressionsInternal

2020-03-17 Thread Danny Chan
I’m a little worried about it the default RexExecutorImpl can handle all the downstream projects expressions, and very probably not, there would be some Janino compile exception if it can not translate the RexNodes correctly. So strictly to say, change the RexExecutor to a default implementation

Re: [DISCUSS] get RexExecutor from RexSimplify in method reduceExpressionsInternal

2020-03-17 Thread Stamatis Zampetakis
I don't know what others think but +1 from my side. Best, Stamatis On Tue, Mar 17, 2020 at 9:16 AM JiaTao Tao wrote: > Hi Danny > > Thanks for your reply, I think Stamatis Zampetakis's opinion is summative, > and here the problem I think is a default RexExecutor is better than null, > especiall

Re: [DISCUSS] get RexExecutor from RexSimplify in method reduceExpressionsInternal

2020-03-17 Thread JiaTao Tao
Hi Danny Thanks for your reply, I think Stamatis Zampetakis's opinion is summative, and here the problem I think is a default RexExecutor is better than null, especially, in this case, cuz `reduceExpressionsInternal` and `reduceExpressions` is in the same path, thought the use of RexExecutor may b

Re: [DISCUSS] get RexExecutor from RexSimplify in method reduceExpressionsInternal

2020-03-17 Thread JiaTao Tao
Hi Stamatis Zampetakis I agree with this completely: "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." Regards! Aron Tao Stamatis Zampetakis 于2020年3月16日周一 下午9:52写道: > In

Re: [DISCUSS] get RexExecutor from RexSimplify in method reduceExpressionsInternal

2020-03-16 Thread Stamatis Zampetakis
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()

Re: [DISCUSS] get RexExecutor from RexSimplify in method reduceExpressionsInternal

2020-03-16 Thread Danny Chan
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