Aman Sinha has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17168 )

Change subject: WIP IMPALA-10564: Return error when inserting an overflowed 
decimal value
......................................................................


Patch Set 1: Code-Review+1

(2 comments)

Code change LGTM. Have a couple of comments. I can upgrade to +2 once you add a 
unit test and address comments.
One other question is about non-HDFS tables. Is insert into Kudu tables covered 
through these ? You don't have to test it for this fix but something to check 
in the future.

http://gerrit.cloudera.org:8080/#/c/17168/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17168/1//COMMIT_MSG@7
PS1, Line 7: overflowed
See related comment below.. it can occur for other error conditions besides 
overflow so you can change the message to be something more generic..like 
'inserting an invalid decimal value'


http://gerrit.cloudera.org:8080/#/c/17168/1/be/src/exec/hdfs-text-table-writer.cc
File be/src/exec/hdfs-text-table-writer.cc:

http://gerrit.cloudera.org:8080/#/c/17168/1/be/src/exec/hdfs-text-table-writer.cc@107
PS1, Line 107: overflowed
It may not always be overflow.  For instance I get the same NULL value inserted 
in the table for a divide-by-zero error:

insert into t1 select cast((a+b+c)/0 as decimal (28,10)) as x from (select 
cast(654964569154.9565 as decimal (28,7)) as a, cast(44658554984 as decimal 
(28,7)) as b, cast(2.111 as decimal (28,7)) as c) q;

I see these are the error conditions for decimal :

grep SetError exprs/decimal-operators-ir.cc

      ctx->SetError("Decimal expression overflowed"); \
      ctx->SetError("String to Decimal parse failed");
      ctx->SetError("String to Decimal cast overflowed");
          if (decimal_v2) ctx->SetError("Cannot divide decimal by zero"); \



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I64ce4ed194af81ef06401ffc1124e12f05b8da98
Gerrit-Change-Number: 17168
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <[email protected]>
Gerrit-Reviewer: Aman Sinha <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]>
Gerrit-Comment-Date: Wed, 10 Mar 2021 03:16:45 +0000
Gerrit-HasComments: Yes

Reply via email to