Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/17168 )
Change subject: IMPALA-10564: Return error when inserting an invalid decimal value ...................................................................... Patch Set 4: Code-Review+1 (7 comments) Thanks for making the changes. A few mostly nits and one comment about the test. 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: lid decima > Done Done 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: invalid de > Done Done http://gerrit.cloudera.org:8080/#/c/17168/4/be/src/exec/hdfs-text-table-writer.cc File be/src/exec/hdfs-text-table-writer.cc: http://gerrit.cloudera.org:8080/#/c/17168/4/be/src/exec/hdfs-text-table-writer.cc@107 PS4, Line 107: // IMPALA-10564: For invalid decimal value, we should return an error nit: the comment indicates this is done unconditionally. Could you update it similar to other places that it is done based on the query option. http://gerrit.cloudera.org:8080/#/c/17168/4/be/src/exec/kudu-table-sink.cc File be/src/exec/kudu-table-sink.cc: http://gerrit.cloudera.org:8080/#/c/17168/4/be/src/exec/kudu-table-sink.cc@253 PS4, Line 253: // IMPALA-10564: For invalid decimal value, we should return an error nit: same comment as above. http://gerrit.cloudera.org:8080/#/c/17168/4/be/src/exec/parquet/hdfs-parquet-table-writer.cc File be/src/exec/parquet/hdfs-parquet-table-writer.cc: http://gerrit.cloudera.org:8080/#/c/17168/4/be/src/exec/parquet/hdfs-parquet-table-writer.cc@698 PS4, Line 698: // IMPALA-10564: For invalid decimal value, we should return an error nit: same as above http://gerrit.cloudera.org:8080/#/c/17168/4/common/thrift/generate_error_codes.py File common/thrift/generate_error_codes.py: http://gerrit.cloudera.org:8080/#/c/17168/4/common/thrift/generate_error_codes.py@475 PS4, Line 475: type 'value' would be more accurate than 'type' to avoid confusion with decimal type precision and scale. http://gerrit.cloudera.org:8080/#/c/17168/4/testdata/workloads/functional-query/queries/QueryTest/decimal-insert-overflow-exprs.test File testdata/workloads/functional-query/queries/QueryTest/decimal-insert-overflow-exprs.test: http://gerrit.cloudera.org:8080/#/c/17168/4/testdata/workloads/functional-query/queries/QueryTest/decimal-insert-overflow-exprs.test@43 PS4, Line 43: ---- RESULTS This verifies if a row was inserted but Is there a way to check that the inserted value is NULL ? I was thinking of something like 'select count(*) from overflowed_decimal_tbl where <column> is null' . Although, since you are using the same table for the insertions, the count will keep growing. -- 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: 4 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-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Comment-Date: Thu, 18 Mar 2021 23:18:54 +0000 Gerrit-HasComments: Yes
