You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Daniel Becker (Code Review)" <ge...@cloudera.org> on 2023/05/08 16:07:23 UTC
[Impala-ASF-CR] IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
Daniel Becker has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19856
Change subject: IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
......................................................................
IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
The statement
select cast(9999999999999999999999 as BIGINT)
correctly fails with the following message:
ERROR: UDF ERROR: Decimal expression overflowed
However, if the number is much bigger, no error is produced:
select cast(999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999
as BIGINT)
The root cause of the problem is this:
- In the first case, the literal (9999999999999999999999) can fit into a
DECIMAL and therefore is interpreted as such. This DECIMAL can't fit
into a BIGINT, and this is checked, so the cast fails.
- In the second case, the literal can't fit in a DECIMAL, so it is
interpreted as a DOUBLE instead. This is not unreasonable as it fits
in the range of DOUBLE, although of course there may be loss of
precision. However, there is no check when casting from DOUBLE to
BIGINT, and because the value is out of range for BIGINT this invokes
undefined behaviour (probably leading to an incorrect value).
This change adds checks to casts
- from floating point types to integer types
- from double to float
These casts can invoke undefined behaviour in C++ and were not checked
before this change.
If the checks fail, an ERROR is raised.
Testing:
- Added unit tests in expr-tests.cc for the new checks
Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
---
M be/src/exprs/cast-functions-ir.cc
M be/src/exprs/cast-functions.h
M be/src/exprs/expr-test.cc
M be/src/exprs/literal.cc
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
5 files changed, 329 insertions(+), 32 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/56/19856/2
--
To view, visit http://gerrit.cloudera.org:8080/19856
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
Gerrit-Change-Number: 19856
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
[Impala-ASF-CR] IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19856 )
Change subject: IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/19856/2/be/src/exprs/cast-functions-ir.cc
File be/src/exprs/cast-functions-ir.cc:
http://gerrit.cloudera.org:8080/#/c/19856/2/be/src/exprs/cast-functions-ir.cc@179
PS2, Line 179: // 1. If the source value can be represented exactly in the destination type, it does not
line too long (92 > 90)
http://gerrit.cloudera.org:8080/#/c/19856/2/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:
http://gerrit.cloudera.org:8080/#/c/19856/2/be/src/exprs/expr-test.cc@3624
PS2, Line 3624: const double next_double = std::nextafter(float_max, std::numeric_limits<double>::max());
line too long (91 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/19856
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
Gerrit-Change-Number: 19856
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 May 2023 16:08:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/19856 )
Change subject: IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
......................................................................
IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
The statement
select cast(9999999999999999999999 as BIGINT)
correctly fails with the following message:
ERROR: UDF ERROR: Decimal expression overflowed
However, if the number is much bigger, no error is produced:
select cast(999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999
as BIGINT)
The root cause of the problem is this:
- In the first case, the literal (9999999999999999999999) can fit into a
DECIMAL and therefore is interpreted as such. This DECIMAL can't fit
into a BIGINT, and this is checked, so the cast fails.
- In the second case, the literal can't fit in a DECIMAL, so it is
interpreted as a DOUBLE instead. This is not unreasonable as it fits
in the range of DOUBLE, although of course there may be loss of
precision. However, there is no check when casting from DOUBLE to
BIGINT, and because the value is out of range for BIGINT this invokes
undefined behaviour (probably leading to an incorrect value).
This change adds checks to casts
- from floating point types to integer types
- from double to float
These casts can invoke undefined behaviour in C++ and were not checked
before this change.
If the checks fail, an ERROR is raised.
Testing:
- Added unit tests in expr-tests.cc for the new checks
Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
---
M be/src/exprs/cast-functions-ir.cc
M be/src/exprs/cast-functions.h
M be/src/exprs/expr-test.cc
M be/src/exprs/literal.cc
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
5 files changed, 328 insertions(+), 40 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/56/19856/4
--
To view, visit http://gerrit.cloudera.org:8080/19856
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
Gerrit-Change-Number: 19856
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
[Impala-ASF-CR] IMPALA-12035: Impala accepts very big numbers but fails to store them correctly
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19856 )
Change subject: IMPALA-12035: Impala accepts very big numbers but fails to store them correctly
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/19856
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
Gerrit-Change-Number: 19856
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 May 2023 16:04:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12035: Impala accepts very big numbers but fails to store them correctly
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/19856 )
Change subject: IMPALA-12035: Impala accepts very big numbers but fails to store them correctly
......................................................................
IMPALA-12035: Impala accepts very big numbers but fails to store them correctly
The statement
select cast(9999999999999999999999 as BIGINT)
correctly fails with the following message:
ERROR: UDF ERROR: Decimal expression overflowed
However, if the number is much bigger, no error is produced:
select cast(999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999
as BIGINT)
The root cause of the problem is this:
- In the first case, the literal (9999999999999999999999) can fit into a
DECIMAL and therefore is interpreted as such. This DECIMAL can't fit
into a BIGINT, and this is checked, so the cast fails.
- In the second case, the literal can't fit in a DECIMAL, so it is
interpreted as a DOUBLE instead. This is not unreasonable as it fits
in the range of DOUBLE, although of course there may be loss of
precision. However, there is no check when casting from DOUBLE to
BIGINT, and because the value is out of range for BIGINT this invokes
undefined behaviour (probably leading to an incorrect value).
This change adds checks to casts
- from floating point types to integer types
- from double to float
These casts can invoke undefined behaviour in C++ and were not checked
before this change.
If the checks fail, an ERROR is raised.
Testing:
- Added unit tests in expr-tests.cc for the new checks
Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
---
M be/src/exprs/cast-functions-ir.cc
M be/src/exprs/cast-functions.h
M be/src/exprs/expr-test.cc
M be/src/exprs/literal.cc
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
5 files changed, 328 insertions(+), 40 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/56/19856/5
--
To view, visit http://gerrit.cloudera.org:8080/19856
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
Gerrit-Change-Number: 19856
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
[Impala-ASF-CR] IMPALA-12035: Impala accepts very big numbers but fails to store them correctly
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/19856 )
Change subject: IMPALA-12035: Impala accepts very big numbers but fails to store them correctly
......................................................................
IMPALA-12035: Impala accepts very big numbers but fails to store them correctly
The statement
select cast(9999999999999999999999 as BIGINT)
correctly fails with the following message:
ERROR: UDF ERROR: Decimal expression overflowed
However, if the number is much bigger, no error is produced:
select cast(999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999
as BIGINT)
The root cause of the problem is this:
- In the first case, the literal (9999999999999999999999) can fit into a
DECIMAL and therefore is interpreted as such. This DECIMAL can't fit
into a BIGINT, and this is checked, so the cast fails.
- In the second case, the literal can't fit in a DECIMAL, so it is
interpreted as a DOUBLE instead. This is not unreasonable as it fits
in the range of DOUBLE, although of course there may be loss of
precision. However, there is no check when casting from DOUBLE to
BIGINT, and because the value is out of range for BIGINT this invokes
undefined behaviour (probably leading to an incorrect value).
This change adds checks to casts
- from floating point types to integer types
- from double to float
These casts can invoke undefined behaviour in C++ and were not checked
before this change.
If the checks fail, an ERROR is raised.
Testing:
- Added unit tests in expr-tests.cc for the new checks
Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
Reviewed-on: http://gerrit.cloudera.org:8080/19856
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/exprs/cast-functions-ir.cc
M be/src/exprs/cast-functions.h
M be/src/exprs/expr-test.cc
M be/src/exprs/literal.cc
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
5 files changed, 328 insertions(+), 40 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/19856
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
Gerrit-Change-Number: 19856
Gerrit-PatchSet: 7
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
[Impala-ASF-CR] IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19856 )
Change subject: IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
......................................................................
Patch Set 3:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/12970/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/19856
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
Gerrit-Change-Number: 19856
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 May 2023 09:59:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12035: Impala accepts very big numbers but fails to store them correctly
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19856 )
Change subject: IMPALA-12035: Impala accepts very big numbers but fails to store them correctly
......................................................................
Patch Set 6:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9306/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/19856
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
Gerrit-Change-Number: 19856
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 May 2023 16:04:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/19856 )
Change subject: IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/19856
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
Gerrit-Change-Number: 19856
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 May 2023 17:18:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12035: Impala accepts very big numbers but fails to store them correctly
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19856 )
Change subject: IMPALA-12035: Impala accepts very big numbers but fails to store them correctly
......................................................................
Patch Set 6: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/19856
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
Gerrit-Change-Number: 19856
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 May 2023 21:28:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19856 )
Change subject: IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
......................................................................
Patch Set 4: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19856/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19856/4//COMMIT_MSG@7
PS4, Line 7: shell
Can you remove the "shell" part? This is not specific to impala-shell
--
To view, visit http://gerrit.cloudera.org:8080/19856
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
Gerrit-Change-Number: 19856
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 May 2023 15:53:38 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12035: Impala accepts very big numbers but fails to store them correctly
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19856 )
Change subject: IMPALA-12035: Impala accepts very big numbers but fails to store them correctly
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13003/ : 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/19856
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
Gerrit-Change-Number: 19856
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 May 2023 16:24:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12035: Impala accepts very big numbers but fails to store them correctly
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19856 )
Change subject: IMPALA-12035: Impala accepts very big numbers but fails to store them correctly
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19856/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19856/5//COMMIT_MSG@7
PS5, Line 7: Impala
Modified the commit message because the problem is not specific to impala-shell.
--
To view, visit http://gerrit.cloudera.org:8080/19856
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
Gerrit-Change-Number: 19856
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 May 2023 16:03:01 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19856 )
Change subject: IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12986/ : 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/19856
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
Gerrit-Change-Number: 19856
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 May 2023 09:33:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/19856 )
Change subject: IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
......................................................................
Patch Set 2:
(2 comments)
Please add a test that demonstrates that a query with out-of-range cast fails.
http://gerrit.cloudera.org:8080/#/c/19856/2/be/src/exprs/cast-functions-ir.cc
File be/src/exprs/cast-functions-ir.cc:
http://gerrit.cloudera.org:8080/#/c/19856/2/be/src/exprs/cast-functions-ir.cc@120
PS2, Line 120: bool Validate(FROM_TYPE from, FunctionContext* ctx) {
Why isn't this static?
http://gerrit.cloudera.org:8080/#/c/19856/2/be/src/exprs/cast-functions-ir.cc@211
PS2, Line 211: const bool valid = Validate<from_underlying_type, to_underlying_type>(val.val, ctx); \
I'm surprised the types can't be inferred here.
--
To view, visit http://gerrit.cloudera.org:8080/19856
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
Gerrit-Change-Number: 19856
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 May 2023 18:21:06 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/19856 )
Change subject: IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
......................................................................
Patch Set 3: Code-Review+1
(2 comments)
> Patch Set 3:
>
> (5 comments)
>
> Michael, do you mean end-to-end tests? I've added BE unit tests in expr-test.cc in ExprTest.CastFloatingPointNarrowing() that are almost like actual queries - do you think that these should be duplicated as EE tests? Or just a few of them?
Oh right, the BE unit test should be fine.
http://gerrit.cloudera.org:8080/#/c/19856/2/be/src/exprs/cast-functions-ir.cc
File be/src/exprs/cast-functions-ir.cc:
http://gerrit.cloudera.org:8080/#/c/19856/2/be/src/exprs/cast-functions-ir.cc@120
PS2, Line 120: bool Validate(FROM_TYPE from, FunctionContext* ctx) {
> It's not a class method and this is in an anonymous namespace, so it's only
It stands out from all the other versions of Validate, which are marked static.
http://gerrit.cloudera.org:8080/#/c/19856/2/be/src/exprs/cast-functions-ir.cc@211
PS2, Line 211: const bool valid = Validate<from_underlying_type, to_underlying_type>(val.val, ctx); \
> The FROM type could probably be inferred but the TO type is not connected t
Ack
--
To view, visit http://gerrit.cloudera.org:8080/19856
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
Gerrit-Change-Number: 19856
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 May 2023 16:14:00 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19856 )
Change subject: IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
......................................................................
Patch Set 3:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/12971/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/19856
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
Gerrit-Change-Number: 19856
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 May 2023 13:13:19 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/19856 )
Change subject: IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
......................................................................
IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
The statement
select cast(9999999999999999999999 as BIGINT)
correctly fails with the following message:
ERROR: UDF ERROR: Decimal expression overflowed
However, if the number is much bigger, no error is produced:
select cast(999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999
as BIGINT)
The root cause of the problem is this:
- In the first case, the literal (9999999999999999999999) can fit into a
DECIMAL and therefore is interpreted as such. This DECIMAL can't fit
into a BIGINT, and this is checked, so the cast fails.
- In the second case, the literal can't fit in a DECIMAL, so it is
interpreted as a DOUBLE instead. This is not unreasonable as it fits
in the range of DOUBLE, although of course there may be loss of
precision. However, there is no check when casting from DOUBLE to
BIGINT, and because the value is out of range for BIGINT this invokes
undefined behaviour (probably leading to an incorrect value).
This change adds checks to casts
- from floating point types to integer types
- from double to float
These casts can invoke undefined behaviour in C++ and were not checked
before this change.
If the checks fail, an ERROR is raised.
Testing:
- Added unit tests in expr-tests.cc for the new checks
Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
---
M be/src/exprs/cast-functions-ir.cc
M be/src/exprs/cast-functions.h
M be/src/exprs/expr-test.cc
M be/src/exprs/literal.cc
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
5 files changed, 327 insertions(+), 39 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/56/19856/3
--
To view, visit http://gerrit.cloudera.org:8080/19856
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
Gerrit-Change-Number: 19856
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
[Impala-ASF-CR] IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19856 )
Change subject: IMPALA-12035: impala-shell accepts very big numbers but fails to store them correctly
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12964/ : 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/19856
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd97dee3c793a5caa95b1fe5d22b27b8c0f8275d
Gerrit-Change-Number: 19856
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 May 2023 16:29:14 +0000
Gerrit-HasComments: No