Dan Hecht has posted comments on this change. Change subject: Inject precision and scale constants for all decimal builtins ......................................................................
Patch Set 4: (6 comments) http://gerrit.cloudera.org:8080/#/c/2535/4/be/src/exprs/decimal-functions.cc File be/src/exprs/decimal-functions.cc: Line 64: cross-compiled the other similar comments don't say "cross-compiled". let's be consistent one way or the other. http://gerrit.cloudera.org:8080/#/c/2535/4/be/src/exprs/expr.h File be/src/exprs/expr.h: Line 265: any recognized ? Line 267: // work as-is. i think explaining where IR_ALWAYS_INLINE needs to be used in order to make this work is worthwhile. Line 281: . and return type. and if it's truly a helper for those, then it should be private. Line 282: I think we usually put a newline here. Line 288: /// provided arguments. Returns the number of calls replaced. separate out these comments to make it more explicit what the difference between them. how does one decide which one should be used? -- 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: 4 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Skye Wanderman-Milne <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
