Tim Armstrong has posted comments on this change. Change subject: Inject precision and scale constants for all decimal builtins ......................................................................
Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/2535/8/be/src/exprs/expr.cc File be/src/exprs/expr.cc: Line 614: case ARG_TYPE_SCALE: > but what's an example of a non-int constant that would be derived from arg_ There was an alternate version of the patch where we had a variant returning a TypeDesc struct where that made more sense. As discussed, I ended up simplifying the logic to remove the templating and avoid the unnecessary switch. I think we'll need to revisit this mechanism to support constants in places other than expressions so it's better to keep it as simple as possible for now. One advantage of the templating approach was the mangled symbols had the same prefix independent of type. I think we may want to have different namespaces anyway, e.g. HashTable::GetConstant(), so this may not be much of an advantage. -- To view, visit http://gerrit.cloudera.org:8080/2535 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7b7d1485f357ba88517ca15c5c7428c0ffe25dfd Gerrit-PatchSet: 8 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Huaisi Xu <[email protected]> Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Skye Wanderman-Milne <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
