Dan Hecht has posted comments on this change. Change subject: Inject precision and scale constants for all decimal builtins ......................................................................
Patch Set 10: Code-Review+2 (2 comments) http://gerrit.cloudera.org:8080/#/c/2535/10/be/src/exprs/expr.cc File be/src/exprs/expr.cc: Line 610: GetConstantInt(return_type, arg_types, c, i)); Consider even getting rid of GetIrConstant and just putting htis code in InlineConstants. I think having less helper routines makes it easier to read (and as you mentioned we're going to have to more drastically refactor the code to handle other cases anyway). But okay to ignore if you prefer having GetIrConstant. http://gerrit.cloudera.org:8080/#/c/2535/10/be/src/exprs/expr.h File be/src/exprs/expr.h: Line 456: Get*Constant did you mean GetConstant*()? -- 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: 10 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
