You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Amogh Margoor (Code Review)" <ge...@cloudera.org> on 2021/04/11 17:39:06 UTC
[Impala-ASF-CR] IMPALA-10350 Fix precision loss while converting DecimalValue to double.
Amogh Margoor has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17303
Change subject: IMPALA-10350 Fix precision loss while converting DecimalValue to double.
......................................................................
IMPALA-10350 Fix precision loss while converting DecimalValue to double.
Original approach to convert DecimalValue(internal representation of decimals) was not accurate.
It was:
static_cast<double>(value_) / pow(10.0, scale).
However only integers from −2^53 to 2^53 can be represented accurately by
double precision without any loss.
Hence, it would not work for numbers like -0.43149576573887316.
For DecimalValue representing -0.43149576573887316, value_ would be
-43149576573887316 and scale would be 17. As value_ < -2^53, result would
not be accurate. In newer approach we are using third party library
https://github.com/lemire/fast_double_parser, which handles above scenario
in a performant manner.
Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
---
M be/src/runtime/decimal-value.inline.h
A be/src/thirdparty/fast_double_parser/LICENSE
A be/src/thirdparty/fast_double_parser/LICENSE.BSL
A be/src/thirdparty/fast_double_parser/README.md
A be/src/thirdparty/fast_double_parser/fast_double_parser.h
M testdata/workloads/functional-query/queries/QueryTest/values.test
6 files changed, 1,540 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/17303/1
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 1
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
[Impala-ASF-CR] IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
......................................................................
Patch Set 7: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 7
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Apr 2021 14:52:42 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10350 Fix precision loss while converting DecimalValue to double.
Posted by "Amogh Margoor (Code Review)" <ge...@cloudera.org>.
Amogh Margoor has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10350 Fix precision loss while converting DecimalValue to double.
......................................................................
IMPALA-10350 Fix precision loss while converting DecimalValue to double.
Original approach to convert DecimalValue(internal representation of decimals) was not accurate.
It was:
static_cast<double>(value_) / pow(10.0, scale).
However only integers from −2^53 to 2^53 can be represented accurately by
double precision without any loss.
Hence, it would not work for numbers like -0.43149576573887316.
For DecimalValue representing -0.43149576573887316, value_ would be
-43149576573887316 and scale would be 17. As value_ < -2^53, result would
not be accurate. In newer approach we are using third party library
https://github.com/lemire/fast_double_parser, which handles above scenario
in a performant manner.
Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
---
M be/src/runtime/decimal-value.inline.h
A be/src/thirdparty/fast_double_parser/LICENSE
A be/src/thirdparty/fast_double_parser/LICENSE.BSL
A be/src/thirdparty/fast_double_parser/README.md
A be/src/thirdparty/fast_double_parser/fast_double_parser.h
M bin/rat_exclude_files.txt
M testdata/workloads/functional-query/queries/QueryTest/values.test
7 files changed, 1,541 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/17303/3
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 3
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-10350 Fix precision loss while converting DecimalValue to double.
Posted by "Amogh Margoor (Code Review)" <ge...@cloudera.org>.
Amogh Margoor has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10350 Fix precision loss while converting DecimalValue to double.
......................................................................
IMPALA-10350 Fix precision loss while converting DecimalValue to double.
Original approach to convert DecimalValue(internal representation of decimals) was not accurate.
It was:
static_cast<double>(value_) / pow(10.0, scale).
However only integers from −2^53 to 2^53 can be represented accurately by
double precision without any loss.
Hence, it would not work for numbers like -0.43149576573887316.
For DecimalValue representing -0.43149576573887316, value_ would be
-43149576573887316 and scale would be 17. As value_ < -2^53, result would
not be accurate. In newer approach we are using third party library
https://github.com/lemire/fast_double_parser, which handles above scenario
in a performant manner.
Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
---
M be/src/runtime/decimal-value.inline.h
A be/src/thirdparty/fast_double_parser/LICENSE
A be/src/thirdparty/fast_double_parser/LICENSE.BSL
A be/src/thirdparty/fast_double_parser/README.md
A be/src/thirdparty/fast_double_parser/fast_double_parser.h
M bin/rat_exclude_files.txt
M testdata/workloads/functional-query/queries/QueryTest/values.test
7 files changed, 1,551 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/17303/4
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 4
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
......................................................................
Patch Set 6:
(20 comments)
http://gerrit.cloudera.org:8080/#/c/17303/6/be/src/thirdparty/fast_double_parser/fast_double_parser.h
File be/src/thirdparty/fast_double_parser/fast_double_parser.h:
http://gerrit.cloudera.org:8080/#/c/17303/6/be/src/thirdparty/fast_double_parser/fast_double_parser.h@13
PS6, Line 13: #if (defined(sun) || defined(__sun))
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/6/be/src/thirdparty/fast_double_parser/fast_double_parser.h@17
PS6, Line 17: #if defined(__CYGWIN__) || defined(__MINGW32__) || defined(__MINGW64__)
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/6/be/src/thirdparty/fast_double_parser/fast_double_parser.h@22
PS6, Line 22: * Determining whether we should import xlocale.h or not is
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/6/be/src/thirdparty/fast_double_parser/fast_double_parser.h@25
PS6, Line 25: #if defined(FAST_DOUBLE_PARSER_SOLARIS) || defined(FAST_DOUBLE_PARSER_CYGWIN)
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/6/be/src/thirdparty/fast_double_parser/fast_double_parser.h@67
PS6, Line 67: #endif // defined(FAST_DOUBLE_PARSER_SOLARIS) || defined(FAST_DOUBLE_PARSER_CYGWIN)
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/6/be/src/thirdparty/fast_double_parser/fast_double_parser.h@84
PS6, Line 84: * However, we have that
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/6/be/src/thirdparty/fast_double_parser/fast_double_parser.h@86
PS6, Line 86: * Thus it is possible for a number of the form w * 10^-342 where
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/6/be/src/thirdparty/fast_double_parser/fast_double_parser.h@94
PS6, Line 94: * Any number of form w * 10^309 where w>= 1 is going to be
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/6/be/src/thirdparty/fast_double_parser/fast_double_parser.h@153
PS6, Line 153: // credit: https://stackoverflow.com/questions/28868367/getting-the-high-part-of-64-bit-integer-multiplication
line too long (110 > 90)
http://gerrit.cloudera.org:8080/#/c/17303/6/be/src/thirdparty/fast_double_parser/fast_double_parser.h@154
PS6, Line 154: really_inline uint64_t Emulate64x64to128(uint64_t& r_hi, const uint64_t x, const uint64_t y) {
line too long (94 > 90)
http://gerrit.cloudera.org:8080/#/c/17303/6/be/src/thirdparty/fast_double_parser/fast_double_parser.h@159
PS6, Line 159:
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/6/be/src/thirdparty/fast_double_parser/fast_double_parser.h@224
PS6, Line 224: */
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/6/be/src/thirdparty/fast_double_parser/fast_double_parser.h@958
PS6, Line 958:
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/6/be/src/thirdparty/fast_double_parser/fast_double_parser.h@960
PS6, Line 960: // The exponent is 1024 + 63 + power
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/6/be/src/thirdparty/fast_double_parser/fast_double_parser.h@976
PS6, Line 976: // The 65536 is (1<<16) and corresponds to
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/6/be/src/thirdparty/fast_double_parser/fast_double_parser.h@979
PS6, Line 979: // ((152170 * power ) >> 16) is equal to
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/6/be/src/thirdparty/fast_double_parser/fast_double_parser.h@980
PS6, Line 980: // floor(log(5**power)/log(2))
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/6/be/src/thirdparty/fast_double_parser/fast_double_parser.h@982
PS6, Line 982: // Note that this is not magic: 152170/(1<<16) is
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/6/be/src/thirdparty/fast_double_parser/fast_double_parser.h@984
PS6, Line 984: // The 1<<16 value is a power of two; we could use a
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/6/be/src/thirdparty/fast_double_parser/fast_double_parser.h@1097
PS6, Line 1097: #if defined(FAST_DOUBLE_PARSER_SOLARIS) || defined(FAST_DOUBLE_PARSER_CYGWIN)
line has trailing whitespace
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 6
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 26 Apr 2021 12:32:36 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10350 Fix precision loss while converting DecimalValue to double.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10350 Fix precision loss while converting DecimalValue to double.
......................................................................
Patch Set 2:
(20 comments)
http://gerrit.cloudera.org:8080/#/c/17303/2/be/src/thirdparty/fast_double_parser/fast_double_parser.h
File be/src/thirdparty/fast_double_parser/fast_double_parser.h:
http://gerrit.cloudera.org:8080/#/c/17303/2/be/src/thirdparty/fast_double_parser/fast_double_parser.h@13
PS2, Line 13: #if (defined(sun) || defined(__sun))
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/2/be/src/thirdparty/fast_double_parser/fast_double_parser.h@17
PS2, Line 17: #if defined(__CYGWIN__) || defined(__MINGW32__) || defined(__MINGW64__)
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/2/be/src/thirdparty/fast_double_parser/fast_double_parser.h@22
PS2, Line 22: * Determining whether we should import xlocale.h or not is
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/2/be/src/thirdparty/fast_double_parser/fast_double_parser.h@25
PS2, Line 25: #if defined(FAST_DOUBLE_PARSER_SOLARIS) || defined(FAST_DOUBLE_PARSER_CYGWIN)
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/2/be/src/thirdparty/fast_double_parser/fast_double_parser.h@67
PS2, Line 67: #endif // defined(FAST_DOUBLE_PARSER_SOLARIS) || defined(FAST_DOUBLE_PARSER_CYGWIN)
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/2/be/src/thirdparty/fast_double_parser/fast_double_parser.h@84
PS2, Line 84: * However, we have that
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/2/be/src/thirdparty/fast_double_parser/fast_double_parser.h@86
PS2, Line 86: * Thus it is possible for a number of the form w * 10^-342 where
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/2/be/src/thirdparty/fast_double_parser/fast_double_parser.h@94
PS2, Line 94: * Any number of form w * 10^309 where w>= 1 is going to be
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/2/be/src/thirdparty/fast_double_parser/fast_double_parser.h@153
PS2, Line 153: // credit: https://stackoverflow.com/questions/28868367/getting-the-high-part-of-64-bit-integer-multiplication
line too long (110 > 90)
http://gerrit.cloudera.org:8080/#/c/17303/2/be/src/thirdparty/fast_double_parser/fast_double_parser.h@154
PS2, Line 154: really_inline uint64_t Emulate64x64to128(uint64_t& r_hi, const uint64_t x, const uint64_t y) {
line too long (94 > 90)
http://gerrit.cloudera.org:8080/#/c/17303/2/be/src/thirdparty/fast_double_parser/fast_double_parser.h@159
PS2, Line 159:
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/2/be/src/thirdparty/fast_double_parser/fast_double_parser.h@224
PS2, Line 224: */
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/2/be/src/thirdparty/fast_double_parser/fast_double_parser.h@958
PS2, Line 958:
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/2/be/src/thirdparty/fast_double_parser/fast_double_parser.h@960
PS2, Line 960: // The exponent is 1024 + 63 + power
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/2/be/src/thirdparty/fast_double_parser/fast_double_parser.h@976
PS2, Line 976: // The 65536 is (1<<16) and corresponds to
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/2/be/src/thirdparty/fast_double_parser/fast_double_parser.h@979
PS2, Line 979: // ((152170 * power ) >> 16) is equal to
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/2/be/src/thirdparty/fast_double_parser/fast_double_parser.h@980
PS2, Line 980: // floor(log(5**power)/log(2))
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/2/be/src/thirdparty/fast_double_parser/fast_double_parser.h@982
PS2, Line 982: // Note that this is not magic: 152170/(1<<16) is
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/2/be/src/thirdparty/fast_double_parser/fast_double_parser.h@984
PS2, Line 984: // The 1<<16 value is a power of two; we could use a
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/2/be/src/thirdparty/fast_double_parser/fast_double_parser.h@1097
PS2, Line 1097: #if defined(FAST_DOUBLE_PARSER_SOLARIS) || defined(FAST_DOUBLE_PARSER_CYGWIN)
line has trailing whitespace
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 2
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sun, 11 Apr 2021 18:19:27 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10350 Fix precision loss while converting DecimalValue to double.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10350 Fix precision loss while converting DecimalValue to double.
......................................................................
Patch Set 1:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/8542/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 1
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sun, 11 Apr 2021 17:58:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
......................................................................
Patch Set 5:
(20 comments)
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/thirdparty/fast_double_parser/fast_double_parser.h
File be/src/thirdparty/fast_double_parser/fast_double_parser.h:
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/thirdparty/fast_double_parser/fast_double_parser.h@13
PS5, Line 13: #if (defined(sun) || defined(__sun))
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/thirdparty/fast_double_parser/fast_double_parser.h@17
PS5, Line 17: #if defined(__CYGWIN__) || defined(__MINGW32__) || defined(__MINGW64__)
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/thirdparty/fast_double_parser/fast_double_parser.h@22
PS5, Line 22: * Determining whether we should import xlocale.h or not is
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/thirdparty/fast_double_parser/fast_double_parser.h@25
PS5, Line 25: #if defined(FAST_DOUBLE_PARSER_SOLARIS) || defined(FAST_DOUBLE_PARSER_CYGWIN)
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/thirdparty/fast_double_parser/fast_double_parser.h@67
PS5, Line 67: #endif // defined(FAST_DOUBLE_PARSER_SOLARIS) || defined(FAST_DOUBLE_PARSER_CYGWIN)
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/thirdparty/fast_double_parser/fast_double_parser.h@84
PS5, Line 84: * However, we have that
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/thirdparty/fast_double_parser/fast_double_parser.h@86
PS5, Line 86: * Thus it is possible for a number of the form w * 10^-342 where
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/thirdparty/fast_double_parser/fast_double_parser.h@94
PS5, Line 94: * Any number of form w * 10^309 where w>= 1 is going to be
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/thirdparty/fast_double_parser/fast_double_parser.h@153
PS5, Line 153: // credit: https://stackoverflow.com/questions/28868367/getting-the-high-part-of-64-bit-integer-multiplication
line too long (110 > 90)
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/thirdparty/fast_double_parser/fast_double_parser.h@154
PS5, Line 154: really_inline uint64_t Emulate64x64to128(uint64_t& r_hi, const uint64_t x, const uint64_t y) {
line too long (94 > 90)
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/thirdparty/fast_double_parser/fast_double_parser.h@159
PS5, Line 159:
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/thirdparty/fast_double_parser/fast_double_parser.h@224
PS5, Line 224: */
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/thirdparty/fast_double_parser/fast_double_parser.h@958
PS5, Line 958:
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/thirdparty/fast_double_parser/fast_double_parser.h@960
PS5, Line 960: // The exponent is 1024 + 63 + power
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/thirdparty/fast_double_parser/fast_double_parser.h@976
PS5, Line 976: // The 65536 is (1<<16) and corresponds to
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/thirdparty/fast_double_parser/fast_double_parser.h@979
PS5, Line 979: // ((152170 * power ) >> 16) is equal to
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/thirdparty/fast_double_parser/fast_double_parser.h@980
PS5, Line 980: // floor(log(5**power)/log(2))
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/thirdparty/fast_double_parser/fast_double_parser.h@982
PS5, Line 982: // Note that this is not magic: 152170/(1<<16) is
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/thirdparty/fast_double_parser/fast_double_parser.h@984
PS5, Line 984: // The 1<<16 value is a power of two; we could use a
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/thirdparty/fast_double_parser/fast_double_parser.h@1097
PS5, Line 1097: #if defined(FAST_DOUBLE_PARSER_SOLARIS) || defined(FAST_DOUBLE_PARSER_CYGWIN)
line has trailing whitespace
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 5
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 13 Apr 2021 19:38:33 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
Posted by "Amogh Margoor (Code Review)" <ge...@cloudera.org>.
Amogh Margoor has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
......................................................................
IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
Original approach to convert DecimalValue(internal representation
of decimals) to double was not accurate.
It was:
static_cast<double>(value_) / pow(10.0, scale).
However only integers from −2^53 to 2^53 can be represented
accurately by double precision without any loss.
Hence, it would not work for numbers like -0.43149576573887316.
For DecimalValue representing -0.43149576573887316, value_ would be
-43149576573887316 and scale would be 17. As value_ < -2^53,
result would not be accurate. In newer approach we are using third
party library https://github.com/lemire/fast_double_parser, which
handles above scenario in a performant manner.
Testing:
1. Added End to End Tests covering following scenarios:
a. Test to show precision limitation of 16 in the write path
b. DecimalValue's value_ between -2^53 and 2^53.
b. value_ outside above range but abs(value_) < UINT64_MAX
c. abs(value_) > UINT64_MAX -covers DecimalValue<__int128_t>
2. Ran existing backend and end-to-end tests completely
Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
---
M be/src/runtime/decimal-value.inline.h
A be/src/thirdparty/fast_double_parser/LICENSE
A be/src/thirdparty/fast_double_parser/LICENSE.BSL
A be/src/thirdparty/fast_double_parser/README.md
A be/src/thirdparty/fast_double_parser/fast_double_parser.h
M bin/rat_exclude_files.txt
M testdata/workloads/functional-query/queries/QueryTest/values.test
7 files changed, 1,568 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/17303/5
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 5
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
......................................................................
Patch Set 6: Code-Review+2
Great work, LGTM!
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 6
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Apr 2021 08:26:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
......................................................................
Patch Set 7:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7105/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 7
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Apr 2021 08:27:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10350 Fix precision loss while converting DecimalValue to double.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10350 Fix precision loss while converting DecimalValue to double.
......................................................................
Patch Set 4:
(20 comments)
http://gerrit.cloudera.org:8080/#/c/17303/4/be/src/thirdparty/fast_double_parser/fast_double_parser.h
File be/src/thirdparty/fast_double_parser/fast_double_parser.h:
http://gerrit.cloudera.org:8080/#/c/17303/4/be/src/thirdparty/fast_double_parser/fast_double_parser.h@13
PS4, Line 13: #if (defined(sun) || defined(__sun))
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/4/be/src/thirdparty/fast_double_parser/fast_double_parser.h@17
PS4, Line 17: #if defined(__CYGWIN__) || defined(__MINGW32__) || defined(__MINGW64__)
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/4/be/src/thirdparty/fast_double_parser/fast_double_parser.h@22
PS4, Line 22: * Determining whether we should import xlocale.h or not is
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/4/be/src/thirdparty/fast_double_parser/fast_double_parser.h@25
PS4, Line 25: #if defined(FAST_DOUBLE_PARSER_SOLARIS) || defined(FAST_DOUBLE_PARSER_CYGWIN)
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/4/be/src/thirdparty/fast_double_parser/fast_double_parser.h@67
PS4, Line 67: #endif // defined(FAST_DOUBLE_PARSER_SOLARIS) || defined(FAST_DOUBLE_PARSER_CYGWIN)
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/4/be/src/thirdparty/fast_double_parser/fast_double_parser.h@84
PS4, Line 84: * However, we have that
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/4/be/src/thirdparty/fast_double_parser/fast_double_parser.h@86
PS4, Line 86: * Thus it is possible for a number of the form w * 10^-342 where
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/4/be/src/thirdparty/fast_double_parser/fast_double_parser.h@94
PS4, Line 94: * Any number of form w * 10^309 where w>= 1 is going to be
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/4/be/src/thirdparty/fast_double_parser/fast_double_parser.h@153
PS4, Line 153: // credit: https://stackoverflow.com/questions/28868367/getting-the-high-part-of-64-bit-integer-multiplication
line too long (110 > 90)
http://gerrit.cloudera.org:8080/#/c/17303/4/be/src/thirdparty/fast_double_parser/fast_double_parser.h@154
PS4, Line 154: really_inline uint64_t Emulate64x64to128(uint64_t& r_hi, const uint64_t x, const uint64_t y) {
line too long (94 > 90)
http://gerrit.cloudera.org:8080/#/c/17303/4/be/src/thirdparty/fast_double_parser/fast_double_parser.h@159
PS4, Line 159:
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/4/be/src/thirdparty/fast_double_parser/fast_double_parser.h@224
PS4, Line 224: */
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/4/be/src/thirdparty/fast_double_parser/fast_double_parser.h@958
PS4, Line 958:
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/4/be/src/thirdparty/fast_double_parser/fast_double_parser.h@960
PS4, Line 960: // The exponent is 1024 + 63 + power
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/4/be/src/thirdparty/fast_double_parser/fast_double_parser.h@976
PS4, Line 976: // The 65536 is (1<<16) and corresponds to
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/4/be/src/thirdparty/fast_double_parser/fast_double_parser.h@979
PS4, Line 979: // ((152170 * power ) >> 16) is equal to
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/4/be/src/thirdparty/fast_double_parser/fast_double_parser.h@980
PS4, Line 980: // floor(log(5**power)/log(2))
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/4/be/src/thirdparty/fast_double_parser/fast_double_parser.h@982
PS4, Line 982: // Note that this is not magic: 152170/(1<<16) is
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/4/be/src/thirdparty/fast_double_parser/fast_double_parser.h@984
PS4, Line 984: // The 1<<16 value is a power of two; we could use a
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/4/be/src/thirdparty/fast_double_parser/fast_double_parser.h@1097
PS4, Line 1097: #if defined(FAST_DOUBLE_PARSER_SOLARIS) || defined(FAST_DOUBLE_PARSER_CYGWIN)
line has trailing whitespace
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 4
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 12 Apr 2021 18:59:23 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10350 Fix precision loss while converting DecimalValue to double.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10350 Fix precision loss while converting DecimalValue to double.
......................................................................
Patch Set 4:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/8554/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 4
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 12 Apr 2021 19:18:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10350 Fix precision loss while converting DecimalValue to double.
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10350 Fix precision loss while converting DecimalValue to double.
......................................................................
Patch Set 3:
(7 comments)
The change looks great overall!
http://gerrit.cloudera.org:8080/#/c/17303/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17303/3//COMMIT_MSG@7
PS3, Line 7: IMPALA-10350
Please add : after IMPALA-10350
http://gerrit.cloudera.org:8080/#/c/17303/3//COMMIT_MSG@9
PS3, Line 9: imals) was not accurate.
In commit message bodies we use lines with 72 chars width.
http://gerrit.cloudera.org:8080/#/c/17303/3//COMMIT_MSG@20
PS3, Line 20:
Please add section about testing.
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/runtime/decimal-value.inline.h
File be/src/runtime/decimal-value.inline.h:
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/runtime/decimal-value.inline.h@727
PS3, Line 727: // Original approach was to use:
Thanks for the detailed comments!
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/runtime/decimal-value.inline.h@745
PS3, Line 745: int64_t> (value_
Type of value_ can be __int128_t. If that's the case and the value is greater than the max value of int64_t, then I think we also want to fallback to the original algorithm.
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/runtime/decimal-value.inline.h@747
PS3, Line 747: // Fallback to original approach. This is not always accurate as described above.
: // Other alternative would be to convert value_ to string and parse it into
: // double using std:strtod. However std::strtod is atleast 4X slower
: // than this approach (https://github.com/lemire/fast_double_parser#sample-results)
: // and that is excluding cost to convert value_ to string.
: if (!success) {
: result = static_cast<double>(value_) / pow(10.0, scale);
: }
I'm happy with the current patch as it is a big step forward, but IMPALA-10350 is about correctness and I think we still want to track that problem in Jira.
How about creating a sub-task for IMPALA-10350 and commit this patch with that Jira ID? We'll close the sub-task once this patch is committed, but leave IMPALA-10350 open so we'll be aware of the limitations. Hopefully we'll find a proper algorithm in the future. (If this patch was using strtod then we'd still need another Jira to track perf improvements possibilities in the future).
http://gerrit.cloudera.org:8080/#/c/17303/3/testdata/workloads/functional-query/queries/QueryTest/values.test
File testdata/workloads/functional-query/queries/QueryTest/values.test:
http://gerrit.cloudera.org:8080/#/c/17303/3/testdata/workloads/functional-query/queries/QueryTest/values.test@106
PS3, Line 106: # insert one negative value < -2^53 * 10^(-17)
Please add test where the value needs to be stored in __int128_t.
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 3
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 12 Apr 2021 15:50:54 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10350 Fix precision loss while converting DecimalValue to double.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10350 Fix precision loss while converting DecimalValue to double.
......................................................................
Patch Set 1:
(30 comments)
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/runtime/decimal-value.inline.h
File be/src/runtime/decimal-value.inline.h:
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/runtime/decimal-value.inline.h@727
PS1, Line 727: // Original approach was to use:
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/runtime/decimal-value.inline.h@728
PS1, Line 728: // static_cast<double>(value_) / pow(10.0, scale).
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/runtime/decimal-value.inline.h@729
PS1, Line 729: // However only integers from −2^53 to 2^53 can be represented accurately by
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/runtime/decimal-value.inline.h@730
PS1, Line 730: // double precision without any loss.
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/runtime/decimal-value.inline.h@731
PS1, Line 731: // Hence, it would not work for numbers like -0.43149576573887316.
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/runtime/decimal-value.inline.h@734
PS1, Line 734: // not be accurate. In newer approach we are using
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/runtime/decimal-value.inline.h@742
PS1, Line 742: // It expects value to be positive even for negative numbers.
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/runtime/decimal-value.inline.h@744
PS1, Line 744: double result = fast_double_parser::compute_float_64((-scale),
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/runtime/decimal-value.inline.h@747
PS1, Line 747: // Fallback to original approach. This is not always accurate as described above.
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/runtime/decimal-value.inline.h@749
PS1, Line 749: // double using std:strtod. However std::strtod is atleast 4X slower
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/thirdparty/fast_double_parser/fast_double_parser.h
File be/src/thirdparty/fast_double_parser/fast_double_parser.h:
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/thirdparty/fast_double_parser/fast_double_parser.h@13
PS1, Line 13: #if (defined(sun) || defined(__sun))
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/thirdparty/fast_double_parser/fast_double_parser.h@17
PS1, Line 17: #if defined(__CYGWIN__) || defined(__MINGW32__) || defined(__MINGW64__)
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/thirdparty/fast_double_parser/fast_double_parser.h@22
PS1, Line 22: * Determining whether we should import xlocale.h or not is
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/thirdparty/fast_double_parser/fast_double_parser.h@25
PS1, Line 25: #if defined(FAST_DOUBLE_PARSER_SOLARIS) || defined(FAST_DOUBLE_PARSER_CYGWIN)
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/thirdparty/fast_double_parser/fast_double_parser.h@67
PS1, Line 67: #endif // defined(FAST_DOUBLE_PARSER_SOLARIS) || defined(FAST_DOUBLE_PARSER_CYGWIN)
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/thirdparty/fast_double_parser/fast_double_parser.h@84
PS1, Line 84: * However, we have that
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/thirdparty/fast_double_parser/fast_double_parser.h@86
PS1, Line 86: * Thus it is possible for a number of the form w * 10^-342 where
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/thirdparty/fast_double_parser/fast_double_parser.h@94
PS1, Line 94: * Any number of form w * 10^309 where w>= 1 is going to be
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/thirdparty/fast_double_parser/fast_double_parser.h@153
PS1, Line 153: // credit: https://stackoverflow.com/questions/28868367/getting-the-high-part-of-64-bit-integer-multiplication
line too long (110 > 90)
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/thirdparty/fast_double_parser/fast_double_parser.h@154
PS1, Line 154: really_inline uint64_t Emulate64x64to128(uint64_t& r_hi, const uint64_t x, const uint64_t y) {
line too long (94 > 90)
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/thirdparty/fast_double_parser/fast_double_parser.h@159
PS1, Line 159:
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/thirdparty/fast_double_parser/fast_double_parser.h@224
PS1, Line 224: */
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/thirdparty/fast_double_parser/fast_double_parser.h@958
PS1, Line 958:
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/thirdparty/fast_double_parser/fast_double_parser.h@960
PS1, Line 960: // The exponent is 1024 + 63 + power
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/thirdparty/fast_double_parser/fast_double_parser.h@976
PS1, Line 976: // The 65536 is (1<<16) and corresponds to
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/thirdparty/fast_double_parser/fast_double_parser.h@979
PS1, Line 979: // ((152170 * power ) >> 16) is equal to
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/thirdparty/fast_double_parser/fast_double_parser.h@980
PS1, Line 980: // floor(log(5**power)/log(2))
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/thirdparty/fast_double_parser/fast_double_parser.h@982
PS1, Line 982: // Note that this is not magic: 152170/(1<<16) is
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/thirdparty/fast_double_parser/fast_double_parser.h@984
PS1, Line 984: // The 1<<16 value is a power of two; we could use a
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/1/be/src/thirdparty/fast_double_parser/fast_double_parser.h@1097
PS1, Line 1097: #if defined(FAST_DOUBLE_PARSER_SOLARIS) || defined(FAST_DOUBLE_PARSER_CYGWIN)
line has trailing whitespace
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 1
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sun, 11 Apr 2021 17:39:54 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
Posted by "Amogh Margoor (Code Review)" <ge...@cloudera.org>.
Amogh Margoor has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
......................................................................
IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
Original approach to convert DecimalValue(internal representation
of decimals) to double was not accurate.
It was:
static_cast<double>(value_) / pow(10.0, scale).
However only integers from −2^53 to 2^53 can be represented
accurately by double precision without any loss.
Hence, it would not work for numbers like -0.43149576573887316.
For DecimalValue representing -0.43149576573887316, value_ would be
-43149576573887316 and scale would be 17. As value_ < -2^53,
result would not be accurate. In newer approach we are using third
party library https://github.com/lemire/fast_double_parser, which
handles above scenario in a performant manner.
Testing:
1. Added End to End Tests covering following scenarios:
a. Test to show precision limitation of 16 in the write path
b. DecimalValue's value_ between -2^53 and 2^53.
b. value_ outside above range but abs(value_) < UINT64_MAX
c. abs(value_) > UINT64_MAX -covers DecimalValue<__int128_t>
2. Ran existing backend and end-to-end tests completely
Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
---
M be/src/runtime/decimal-value.inline.h
A be/src/thirdparty/fast_double_parser/LICENSE
A be/src/thirdparty/fast_double_parser/LICENSE.BSL
A be/src/thirdparty/fast_double_parser/README.md
A be/src/thirdparty/fast_double_parser/fast_double_parser.h
M bin/rat_exclude_files.txt
M testdata/workloads/functional-query/queries/QueryTest/values.test
M tests/query_test/test_insert_parquet.py
8 files changed, 1,579 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/17303/6
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 6
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8636/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 6
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 26 Apr 2021 12:50:56 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
Posted by "Amogh Margoor (Code Review)" <ge...@cloudera.org>.
Amogh Margoor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
......................................................................
Patch Set 5:
> (4 comments)
>
> The change looks great. A test with Parquet would be nice, other
> than that I only found nitpicks.
>
> When you upload a new PS please reply to the comments. Most of the
> time clicking on "Done" is enough. This way we'll know we won't
> left anything open.
I did reply to the comments but didn't know the reply gets saved as draft and needs explicit post later. Sorry about that. I figured that out now, so you might get some old replies too.
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 5
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 26 Apr 2021 12:36:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10350 Fix precision loss while converting DecimalValue to double.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10350 Fix precision loss while converting DecimalValue to double.
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8544/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 3
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sun, 11 Apr 2021 20:28:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10350 Fix precision loss while converting DecimalValue to double.
Posted by "Amogh Margoor (Code Review)" <ge...@cloudera.org>.
Amogh Margoor has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10350 Fix precision loss while converting DecimalValue to double.
......................................................................
IMPALA-10350 Fix precision loss while converting DecimalValue to double.
Original approach to convert DecimalValue(internal representation of decimals) was not accurate.
It was:
static_cast<double>(value_) / pow(10.0, scale).
However only integers from −2^53 to 2^53 can be represented accurately by
double precision without any loss.
Hence, it would not work for numbers like -0.43149576573887316.
For DecimalValue representing -0.43149576573887316, value_ would be
-43149576573887316 and scale would be 17. As value_ < -2^53, result would
not be accurate. In newer approach we are using third party library
https://github.com/lemire/fast_double_parser, which handles above scenario
in a performant manner.
Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
---
M be/src/runtime/decimal-value.inline.h
A be/src/thirdparty/fast_double_parser/LICENSE
A be/src/thirdparty/fast_double_parser/LICENSE.BSL
A be/src/thirdparty/fast_double_parser/README.md
A be/src/thirdparty/fast_double_parser/fast_double_parser.h
M testdata/workloads/functional-query/queries/QueryTest/values.test
6 files changed, 1,540 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/17303/2
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 2
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 7
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Apr 2021 08:27:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
......................................................................
Patch Set 5:
(4 comments)
The change looks great. A test with Parquet would be nice, other than that I only found nitpicks.
When you upload a new PS please reply to the comments. Most of the time clicking on "Done" is enough. This way we'll know we won't left anything open.
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/runtime/decimal-value.inline.h
File be/src/runtime/decimal-value.inline.h:
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/runtime/decimal-value.inline.h@753
PS5, Line 753: (
nit: parentheses not needed
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/runtime/decimal-value.inline.h@754
PS5, Line 754:
nit: we indent with 4 spaces if the line belongs to the same statement as the previous line.
http://gerrit.cloudera.org:8080/#/c/17303/5/be/src/runtime/decimal-value.inline.h@755
PS5, Line 755: is_negative, &success
nit: I think we should either put each argument to a separate line, or put all of them to the previous line. In this case I think the latter makes more sense.
http://gerrit.cloudera.org:8080/#/c/17303/5/testdata/workloads/functional-query/queries/QueryTest/values.test
File testdata/workloads/functional-query/queries/QueryTest/values.test:
http://gerrit.cloudera.org:8080/#/c/17303/5/testdata/workloads/functional-query/queries/QueryTest/values.test@104
PS5, Line 104: For write path, the default precision of 16 is a limitation yet to be
That's true for text tables, but could you please add a test with a Parquet table? I think we should getter better precision there.
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 5
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Apr 2021 16:31:32 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10350 Fix precision loss while converting DecimalValue to double.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10350 Fix precision loss while converting DecimalValue to double.
......................................................................
Patch Set 2:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/8543/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 2
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sun, 11 Apr 2021 18:39:01 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
......................................................................
IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
Original approach to convert DecimalValue(internal representation
of decimals) to double was not accurate.
It was:
static_cast<double>(value_) / pow(10.0, scale).
However only integers from −2^53 to 2^53 can be represented
accurately by double precision without any loss.
Hence, it would not work for numbers like -0.43149576573887316.
For DecimalValue representing -0.43149576573887316, value_ would be
-43149576573887316 and scale would be 17. As value_ < -2^53,
result would not be accurate. In newer approach we are using third
party library https://github.com/lemire/fast_double_parser, which
handles above scenario in a performant manner.
Testing:
1. Added End to End Tests covering following scenarios:
a. Test to show precision limitation of 16 in the write path
b. DecimalValue's value_ between -2^53 and 2^53.
b. value_ outside above range but abs(value_) < UINT64_MAX
c. abs(value_) > UINT64_MAX -covers DecimalValue<__int128_t>
2. Ran existing backend and end-to-end tests completely
Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Reviewed-on: http://gerrit.cloudera.org:8080/17303
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/runtime/decimal-value.inline.h
A be/src/thirdparty/fast_double_parser/LICENSE
A be/src/thirdparty/fast_double_parser/LICENSE.BSL
A be/src/thirdparty/fast_double_parser/README.md
A be/src/thirdparty/fast_double_parser/fast_double_parser.h
M bin/rat_exclude_files.txt
M testdata/workloads/functional-query/queries/QueryTest/values.test
M tests/query_test/test_insert_parquet.py
8 files changed, 1,579 insertions(+), 1 deletion(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 8
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10654: Fix precision loss in DecimalValue to double conversion.
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8567/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 5
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 13 Apr 2021 19:58:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10350 Fix precision loss while converting DecimalValue to double.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17303 )
Change subject: IMPALA-10350 Fix precision loss while converting DecimalValue to double.
......................................................................
Patch Set 3:
(20 comments)
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/thirdparty/fast_double_parser/fast_double_parser.h
File be/src/thirdparty/fast_double_parser/fast_double_parser.h:
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/thirdparty/fast_double_parser/fast_double_parser.h@13
PS3, Line 13: #if (defined(sun) || defined(__sun))
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/thirdparty/fast_double_parser/fast_double_parser.h@17
PS3, Line 17: #if defined(__CYGWIN__) || defined(__MINGW32__) || defined(__MINGW64__)
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/thirdparty/fast_double_parser/fast_double_parser.h@22
PS3, Line 22: * Determining whether we should import xlocale.h or not is
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/thirdparty/fast_double_parser/fast_double_parser.h@25
PS3, Line 25: #if defined(FAST_DOUBLE_PARSER_SOLARIS) || defined(FAST_DOUBLE_PARSER_CYGWIN)
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/thirdparty/fast_double_parser/fast_double_parser.h@67
PS3, Line 67: #endif // defined(FAST_DOUBLE_PARSER_SOLARIS) || defined(FAST_DOUBLE_PARSER_CYGWIN)
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/thirdparty/fast_double_parser/fast_double_parser.h@84
PS3, Line 84: * However, we have that
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/thirdparty/fast_double_parser/fast_double_parser.h@86
PS3, Line 86: * Thus it is possible for a number of the form w * 10^-342 where
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/thirdparty/fast_double_parser/fast_double_parser.h@94
PS3, Line 94: * Any number of form w * 10^309 where w>= 1 is going to be
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/thirdparty/fast_double_parser/fast_double_parser.h@153
PS3, Line 153: // credit: https://stackoverflow.com/questions/28868367/getting-the-high-part-of-64-bit-integer-multiplication
line too long (110 > 90)
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/thirdparty/fast_double_parser/fast_double_parser.h@154
PS3, Line 154: really_inline uint64_t Emulate64x64to128(uint64_t& r_hi, const uint64_t x, const uint64_t y) {
line too long (94 > 90)
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/thirdparty/fast_double_parser/fast_double_parser.h@159
PS3, Line 159:
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/thirdparty/fast_double_parser/fast_double_parser.h@224
PS3, Line 224: */
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/thirdparty/fast_double_parser/fast_double_parser.h@958
PS3, Line 958:
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/thirdparty/fast_double_parser/fast_double_parser.h@960
PS3, Line 960: // The exponent is 1024 + 63 + power
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/thirdparty/fast_double_parser/fast_double_parser.h@976
PS3, Line 976: // The 65536 is (1<<16) and corresponds to
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/thirdparty/fast_double_parser/fast_double_parser.h@979
PS3, Line 979: // ((152170 * power ) >> 16) is equal to
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/thirdparty/fast_double_parser/fast_double_parser.h@980
PS3, Line 980: // floor(log(5**power)/log(2))
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/thirdparty/fast_double_parser/fast_double_parser.h@982
PS3, Line 982: // Note that this is not magic: 152170/(1<<16) is
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/thirdparty/fast_double_parser/fast_double_parser.h@984
PS3, Line 984: // The 1<<16 value is a power of two; we could use a
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/17303/3/be/src/thirdparty/fast_double_parser/fast_double_parser.h@1097
PS3, Line 1097: #if defined(FAST_DOUBLE_PARSER_SOLARIS) || defined(FAST_DOUBLE_PARSER_CYGWIN)
line has trailing whitespace
--
To view, visit http://gerrit.cloudera.org:8080/17303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56f0652cb8f81a491b87d9b108a94c00ae6c99a1
Gerrit-Change-Number: 17303
Gerrit-PatchSet: 3
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sun, 11 Apr 2021 20:08:27 +0000
Gerrit-HasComments: Yes