Dan Hecht has posted comments on this change. Change subject: IMPALA-4809: Add Stability for codegen constants ......................................................................
Patch Set 1: IMPALA-4809 isn't about inlining an expression related value, it's just that the way the code is currently factored, Expr routines deal with doing codegen tricks involving FunctionContext's. They really don't belong in Expr in the first place. Michael is in the process of cleaning that up. So, let's not make IMPALA-4809 dependent on this change. I agree a concept like this may be useful and needed in the future, but until we finish cleaning up some surrounding code and have a need for this, I don't think we should introduce this new concept (and it seems more of a codegen related concept than an Expr related concept). As an aside, see Codegen::ReplaceCallSites() and other related code for non-expr places we do constant injection. That's another way you could do 4809, but ultimately you'll need to get at the query options through FunctionContext anyway. -- To view, visit http://gerrit.cloudera.org:8080/5848 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0431bcd84b908bcf130a6d618f57f8f7c5498428 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: No
