Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/8309 )
Change subject: IMPALA-5019: Decimal V2 addition ...................................................................... Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/8309/4/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/8309/4/be/src/exprs/expr-test.cc@a2402 PS4, Line 2402: > Regarding the first point: I'm not sure what the intent of the original test case is and why it was written this way. Zach added this test case as part of the rounding for cast to DECIMAL, so it seems we may be losing some coverage he intended to get. Could you check with Zach to see what he thinks and if we should rewrite this (and other) test cases? If we leave this test case here, I think a comment explaining why 999.5 is the "correct" answer would be helpful, given that CAST(999.55 as DECIMAL(4.1)) should normally result in 999.6 -- To view, visit http://gerrit.cloudera.org:8080/8309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I401049c56d910eb1546a178c909c923b01239336 Gerrit-Change-Number: 8309 Gerrit-PatchSet: 4 Gerrit-Owner: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Zach Amsden <zams...@cloudera.com> Gerrit-Comment-Date: Mon, 20 Nov 2017 22:01:04 +0000 Gerrit-HasComments: Yes
