Tim Armstrong has posted comments on this change. Change subject: IMPALA-4809: Add Stability for codegen constants ......................................................................
Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/5848/1//COMMIT_MSG Commit Message: PS1, Line 16: overloaded > From the codegen perspective, we only care if a value is compilation time c I think this would be useful once we want to do codegen caching and need to reason about whether codegen output is re-usable. Agree we can solve IMPALA-4809 with Expr::GetConstant() or the mechanism in this patch (if that ever makes it in): https://gerrit.cloudera.org/#/c/3843/ http://gerrit.cloudera.org:8080/#/c/5848/1/be/src/exprs/expr.h File be/src/exprs/expr.h: Line 266: RUNTIME_CONSTANT, // Does not change over an execution Concretely, does this mean that it doesn't change within the scope of a process - it can be different if you start another daemon or are on a different system? PS1, Line 266: RUNTIME_CONSTANT > This seems odd to be in Expr class if an example of this class of constants I think it would make sense to move this into a thrift enum - see my other comment about doing the analysis in the FE. Line 267: QUERY_CONSTANT, // Does not change over a single query We may also want to think about how it varies across different Impala daemons. E.g. does QUERY_CONSTANT mean it's the same on all Impala daemons for that query, or just on the current daemon? I think that is effectively a different dimension. I don't think we necessarily need to add more complexity, just that we should document the assumptions. E.g. one thing we've thought about hypothetically is compiling on one daemon then distributing it others. -- 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: Yes
