Zach Amsden has posted comments on this change.

Change subject: IMPALA-2020: Inline big number strings
......................................................................


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5902/12/be/src/runtime/decimal-value.inline.h
File be/src/runtime/decimal-value.inline.h:

Line 42:   // This means the multiplication can cause an unwanted decimal 
overflow.
> What does this mean in terms of regressions when decimal_v2=false? Will we 
There's no regression in precision, this has not changed and both old and new 
versions are equally imprecise.  The old code did the scale check first, and 
the multiply second, which has the potential for overflow.  Now we will at 
least detect the overflow.  Fixing the precision is harder.  We could use long 
doubles or extract this out to an integral type and do the multiplication 
ourselves if we want this to be more precise, but it's not a regression.


-- 
To view, visit http://gerrit.cloudera.org:8080/5902
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5095a366d914cebb0b64bd434a08dbb55c90ed30
Gerrit-PatchSet: 12
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Jim Apple <[email protected]>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Zach Amsden <[email protected]>
Gerrit-HasComments: Yes

Reply via email to