[ 
https://issues.apache.org/jira/browse/CALCITE-4964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17465862#comment-17465862
 ] 

Julian Hyde commented on CALCITE-4964:
--------------------------------------

It seems to me that the benefit is proportional to the average depth of CASE 
branches. If the average depth is 3, then the benefit is 3x. Not worth it if 
this method is only called 10 times while preparing a typical query.

I can see that the method is called for all expressions, not just those with a 
{{CASE}}. That works in your favor, but perhaps those expressions are never 
changed (i.e. {{old}} always equals {{call}}).

Can you add a counter - say a static {{AtomicInteger}} - to that method, and 
see how many times the method is called, and how many times that loop iterates 
during the Calcite suite suite. See how many iterations your revised method 
saves.

And/or devise a test that is much, much slower without your change.

> Reduce recursion when push predicate into CASE
> ----------------------------------------------
>
>                 Key: CALCITE-4964
>                 URL: https://issues.apache.org/jira/browse/CALCITE-4964
>             Project: Calcite
>          Issue Type: Improvement
>            Reporter: Ziwei Liu
>            Assignee: Ziwei Liu
>            Priority: Major
>
> I think CaseShuttle do not need a loop of for(;; ).When a predicate is pushed 
> into case, we only need to check the new RexCall whether need to push 
> predicate into the child operand, not need to do a new recursion.
> So I think CaseShuttle can be reduced like:
> {code:java}
> @Override public RexNode visitCall(RexCall call) {
>    call = (RexCall) super.visitCall(call);
>    if (call instanceof OdpsLambdaRexCall) {
>       return call;
>    }
>    final RexCall old = call;
>    call = ReduceExpressionsRule.pushPredicateIntoCase(call);
>    if (call == old) {
>      return call;
>    } else {
>       boolean containCase = false;
>       List<RexNode> newOps = new ArrayList<>(call.getOperands().size());
>       // call will be like case when c1 then f(x1 ...)
>       // check whether need push f into x1
>       for (int i = 0; i < call.getOperands().size(); i ++) {
>          RexNode op = call.getOperands().get(i);
>          RexNode newOp = op;
>          if (i % 2 == 1 || i == call.getOperands().size() - 1) {
>            if (op instanceof RexCall) {
>              newOp = ReduceExpressionsRule.pushPredicateIntoCase((RexCall) 
> op);
>            }
>          }
>          if (op != newOp) {
>            containCase = true;
>          }
>          newOps.add(newOp);
>        }
>        if (!containCase) {
>          return call;
>        } else {
>          return call.clone(call.getType(), newOps);
>        }
>    }    
> } {code}



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

Reply via email to