Alex Behm has posted comments on this change. Change subject: IMPALA-4848: Add WIDTH_BUCKET() function ......................................................................
Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/6023/5/be/src/exprs/math-functions-ir.cc File be/src/exprs/math-functions-ir.cc: Line 452: Decimal16Value buckets = Decimal16Value::FromInt(input_precision, input_scale, It would be good to summarize the calculations and comment on the chosen types, e.g. why are we using Dec16 here? In general, please move definitions closer to where they are used. The buckets are only used a lot further down where it might become clearer why it should be Dec16. Adding those comments and doing some cleanup will make reviewing much easier. I'll still need to think more carefully about the logic. -- To view, visit http://gerrit.cloudera.org:8080/6023 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: anujphadke <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-Reviewer: anujphadke <[email protected]> Gerrit-HasComments: Yes
