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/11 16:02:59 UTC

[Impala-ASF-CR] IMPALA-12035: Impala accepts very big numbers but fails to store them correctly

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>