You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by gr...@apache.org on 2019/02/11 18:42:45 UTC

[kudu] 03/03: [client] Improve message when setting a decimal on a non-decimal column

This is an automated email from the ASF dual-hosted git repository.

granthenke pushed a commit to branch branch-1.9.x
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 3aae1ba565741ccf2aac6930984639c0062eead0
Author: Grant Henke <gr...@apache.org>
AuthorDate: Mon Feb 11 10:54:41 2019 -0600

    [client] Improve message when setting a decimal on a non-decimal column
    
    This patch improves the error messaging when
    setting a decimal on a non-decimal column.
    
    Changes the order of validation operations to first
    check the column type is valid before checking
    the value is in range.
    
    Change-Id: I331028c3ce88e54eef0a091c0cc98b39293fb3c1
    Reviewed-on: http://gerrit.cloudera.org:8080/12435
    Tested-by: Kudu Jenkins
    Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
 src/kudu/client/scan_batch.cc       |  2 +-
 src/kudu/common/partial_row-test.cc |  6 ++++++
 src/kudu/common/partial_row.cc      | 17 ++++++++++++-----
 3 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/src/kudu/client/scan_batch.cc b/src/kudu/client/scan_batch.cc
index ac63c1f..d79397f 100644
--- a/src/kudu/client/scan_batch.cc
+++ b/src/kudu/client/scan_batch.cc
@@ -341,7 +341,7 @@ Status KuduScanBatch::RowPtr::GetUnscaledDecimal(int col_idx, int128_t* val) con
       return Status::OK();
     default:
       return Status::InvalidArgument(
-          Substitute("invalid type $0 provided for column '$1' (expected DECIMAL)",
+          Substitute("invalid type $0 provided for column '$1' (expected decimal)",
                      col.type_info()->name(), col.name()));
   }
 }
diff --git a/src/kudu/common/partial_row-test.cc b/src/kudu/common/partial_row-test.cc
index fc4671c..5421f1d 100644
--- a/src/kudu/common/partial_row-test.cc
+++ b/src/kudu/common/partial_row-test.cc
@@ -232,6 +232,12 @@ TEST_F(PartialRowTest, UnitTest) {
   EXPECT_EQ("Invalid argument: value -10000.00 out of range for decimal column 'decimal_val'",
             s.ToString());
 
+  // Set a decimal value on a non decimal column.
+  s = row.SetUnscaledDecimal("string_val", 123456);
+  EXPECT_EQ("Invalid argument: invalid type string provided for column "
+            "'string_val' (expected decimal)",
+            s.ToString());
+
   // Even though the storage is actually the same at the moment, we shouldn't be
   // able to set string columns with SetBinary and vice versa.
   EXPECT_FALSE(row.SetBinaryCopy("string_val", "oops").ok());
diff --git a/src/kudu/common/partial_row.cc b/src/kudu/common/partial_row.cc
index a40b98e..cb422f5 100644
--- a/src/kudu/common/partial_row.cc
+++ b/src/kudu/common/partial_row.cc
@@ -301,10 +301,8 @@ Status KuduPartialRow::SetFloat(int col_idx, float val) {
 Status KuduPartialRow::SetDouble(int col_idx, double val) {
   return Set<TypeTraits<DOUBLE> >(col_idx, val);
 }
-Status KuduPartialRow::SetUnscaledDecimal(int col_idx, int128_t val) {
-  const ColumnSchema& col = schema_->column(col_idx);
-  const DataType col_type = col.type_info()->type();
 
+Status CheckDecimalValueInRange(ColumnSchema col, int128_t val) {
   int128_t max_val = MaxUnscaledDecimal(col.type_attributes().precision);
   int128_t min_val = -max_val;
   if (val < min_val || val > max_val) {
@@ -312,16 +310,25 @@ Status KuduPartialRow::SetUnscaledDecimal(int col_idx, int128_t val) {
         Substitute("value $0 out of range for decimal column '$1'",
                    DecimalToString(val, col.type_attributes().scale), col.name()));
   }
+  return Status::OK();
+}
+
+Status KuduPartialRow::SetUnscaledDecimal(int col_idx, int128_t val) {
+  const ColumnSchema& col = schema_->column(col_idx);
+  const DataType col_type = col.type_info()->type();
   switch (col_type) {
     case DECIMAL32:
+      RETURN_NOT_OK(CheckDecimalValueInRange(col, val))
       return Set<TypeTraits<DECIMAL32> >(col_idx, static_cast<int32_t>(val));
     case DECIMAL64:
+      RETURN_NOT_OK(CheckDecimalValueInRange(col, val))
       return Set<TypeTraits<DECIMAL64> >(col_idx, static_cast<int64_t>(val));
     case DECIMAL128:
+      RETURN_NOT_OK(CheckDecimalValueInRange(col, val))
       return Set<TypeTraits<DECIMAL128> >(col_idx, static_cast<int128_t>(val));
     default:
       return Status::InvalidArgument(
-          Substitute("invalid type $0 provided for column '$1' (expected DECIMAL)",
+          Substitute("invalid type $0 provided for column '$1' (expected decimal)",
                      col.type_info()->name(), col.name()));
   }
 }
@@ -700,7 +707,7 @@ Status KuduPartialRow::GetUnscaledDecimal(int col_idx, int128_t *val) const {
       return Status::OK();
     default:
       return Status::InvalidArgument(
-          Substitute("invalid type $0 provided for column '$1' (expected DECIMAL)",
+          Substitute("invalid type $0 provided for column '$1' (expected decimal)",
                      col.type_info()->name(), col.name()));
   }
 }