Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12797 )
Change subject: IMPALA-4356,IMPALA-7331: codegen all ScalarExprs ...................................................................... Patch Set 12: (1 comment) Answered the one question. Will address other comments later. http://gerrit.cloudera.org:8080/#/c/12797/12/be/src/exprs/scalar-fn-call.cc File be/src/exprs/scalar-fn-call.cc: http://gerrit.cloudera.org:8080/#/c/12797/12/be/src/exprs/scalar-fn-call.cc@143 PS12, Line 143: // We're in the interpreted path (i.e. no JIT). Populate our FunctionContext's : // staging_input_vals, which will be reused across calls to scalar_fn_ on the : // interpreted code path.. : vector<AnyVal*>* input_vals = fn_ctx->impl()->staging_input_vals(); : for (int i = 0; i < NumFixedArgs(); ++i) { : AnyVal* input_val; : RETURN_IF_ERROR(AllocateAnyVal(state, eval->expr_perm_pool(), children_[i]->type(), : "Could not allocate expression value", &input_val)); : input_vals->push_back(input_val); : } > Not sure I understand this part. Does it mean that we will do this initiali Yes, the problem I ran into is that code like the below code to GetConstValue() requires that Get*Value() potentially work on all subtrees. This worked before because we filled in function pointers in all subtrees, but broke when we changed that. We could try to be clever and only initialise some subtrees for interpreted execution, but I eventually decided that was an unnecessary microoptimisation and added complexity in trying to reason about which subtrees it was valid to call Get*Value() on. -- To view, visit http://gerrit.cloudera.org:8080/12797 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I839d7a3a2f5e1309c33a1f66013ef11628c5dc11 Gerrit-Change-Number: 12797 Gerrit-PatchSet: 12 Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Thomas Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Fri, 10 May 2019 23:19:27 +0000 Gerrit-HasComments: Yes
