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 2018/02/21 20:01:59 UTC

kudu git commit: KUDU-721: Support range partitions on decimal columns

Repository: kudu
Updated Branches:
  refs/heads/master 723ced836 -> 0815a1238


KUDU-721: Support range partitions on decimal columns

Add the DataType handling to enable range partitions on
decimal columns and a test that fails without the changes.

Also adds range partition tests for edge cases where:
* The upper bound is less than than the lower bound
* The upper bound is equal to the lower bound
* The bounds use MIN/MAX values

Change-Id: I419d442dd233c73166c6e391907c8443ebecc917
Reviewed-on: http://gerrit.cloudera.org:8080/9363
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Kudu Jenkins


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/0815a123
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/0815a123
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/0815a123

Branch: refs/heads/master
Commit: 0815a1238d18e3a82c33018ea07014d202a16018
Parents: 723ced8
Author: Grant Henke <gr...@gmail.com>
Authored: Mon Feb 19 20:45:45 2018 -0600
Committer: Grant Henke <gr...@gmail.com>
Committed: Wed Feb 21 19:40:35 2018 +0000

----------------------------------------------------------------------
 src/kudu/client/client-test.cc     | 122 ++++++++++++++++++++++++++++++--
 src/kudu/common/partition.cc       |  26 ++++++-
 src/kudu/util/decimal_util-test.cc |   8 +++
 src/kudu/util/decimal_util.cc      |   4 ++
 src/kudu/util/decimal_util.h       |   6 +-
 5 files changed, 156 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/0815a123/src/kudu/client/client-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client-test.cc b/src/kudu/client/client-test.cc
index d8993fb..4a44761 100644
--- a/src/kudu/client/client-test.cc
+++ b/src/kudu/client/client-test.cc
@@ -1611,29 +1611,137 @@ TEST_F(ClientTest, TestExclusiveInclusiveUnixTimeMicrosRangeBounds) {
   // Create test table with range partition using non-default bound types.
   // KUDU-1722
   KuduSchemaBuilder builder;
-  KuduSchema u_schema_;
+  KuduSchema schema;
   builder.AddColumn("key")->Type(KuduColumnSchema::UNIXTIME_MICROS)->NotNull()->PrimaryKey();
   builder.AddColumn("value")->Type(KuduColumnSchema::INT32)->NotNull();
-  CHECK_OK(builder.Build(&u_schema_));
-  const string table_name = "TestExclusiveInclusiveUnixTimeMicrosRangeBounds";
-  shared_ptr<KuduTable> table;
+  ASSERT_OK(builder.Build(&schema));
 
-  unique_ptr<KuduPartialRow> lower_bound(u_schema_.NewRow());
+  unique_ptr<KuduPartialRow> lower_bound(schema.NewRow());
   ASSERT_OK(lower_bound->SetUnixTimeMicros("key", -1));
-  unique_ptr<KuduPartialRow> upper_bound(u_schema_.NewRow());
+  unique_ptr<KuduPartialRow> upper_bound(schema.NewRow());
   ASSERT_OK(upper_bound->SetUnixTimeMicros("key", 99));
 
   unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
   table_creator->add_range_partition(lower_bound.release(), upper_bound.release(),
                                       KuduTableCreator::EXCLUSIVE_BOUND,
                                       KuduTableCreator::INCLUSIVE_BOUND);
+
+  const string table_name = "TestExclusiveInclusiveUnixTimeMicrosRangeBounds";
   ASSERT_OK(table_creator->table_name(table_name)
-                          .schema(&u_schema_)
+                          .schema(&schema)
                           .num_replicas(1)
                           .set_range_partition_columns({ "key" })
                           .Create());
 }
 
+TEST_F(ClientTest, TestExclusiveInclusiveDecimalRangeBounds) {
+  KuduSchemaBuilder builder;
+  KuduSchema schema;
+  builder.AddColumn("key")->Type(KuduColumnSchema::DECIMAL)->NotNull()->PrimaryKey()
+      ->Precision(9)->Scale(2);
+  builder.AddColumn("value")->Type(KuduColumnSchema::INT32)->NotNull();
+  ASSERT_OK(builder.Build(&schema));
+
+  unique_ptr<KuduPartialRow> lower_bound(schema.NewRow());
+  ASSERT_OK(lower_bound->SetUnscaledDecimal("key", -1));
+  unique_ptr<KuduPartialRow> upper_bound(schema.NewRow());
+  ASSERT_OK(upper_bound->SetUnscaledDecimal("key", 99));
+
+  unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
+  table_creator->add_range_partition(lower_bound.release(), upper_bound.release(),
+                                     KuduTableCreator::EXCLUSIVE_BOUND,
+                                     KuduTableCreator::INCLUSIVE_BOUND);
+
+  ASSERT_OK(table_creator->table_name("TestExclusiveInclusiveDecimalRangeBounds")
+                .schema(&schema)
+                .num_replicas(1)
+                .set_range_partition_columns({ "key" })
+                .Create());
+}
+
+TEST_F(ClientTest, TestSwappedRangeBounds) {
+  KuduSchemaBuilder builder;
+  KuduSchema schema;
+  builder.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey();
+  builder.AddColumn("value")->Type(KuduColumnSchema::INT32)->NotNull();
+  ASSERT_OK(builder.Build(&schema));
+
+  unique_ptr<KuduPartialRow> lower_bound(schema.NewRow());
+  ASSERT_OK(lower_bound->SetInt32("key", 90));
+  unique_ptr<KuduPartialRow> upper_bound(schema.NewRow());
+  ASSERT_OK(upper_bound->SetInt32("key", -1));
+
+  unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
+  table_creator->add_range_partition(lower_bound.release(), upper_bound.release(),
+                                     KuduTableCreator::EXCLUSIVE_BOUND,
+                                     KuduTableCreator::INCLUSIVE_BOUND);
+
+  Status s = table_creator->table_name("TestSwappedRangeBounds")
+                .schema(&schema)
+                .num_replicas(1)
+                .set_range_partition_columns({ "key" })
+                .Create();
+
+  ASSERT_TRUE(s.IsInvalidArgument());
+  ASSERT_STR_CONTAINS(s.ToString(),
+                      "Error creating table TestSwappedRangeBounds on the master: "
+                          "range partition lower bound must be less than the upper bound");
+}
+
+
+TEST_F(ClientTest, TestEqualRangeBounds) {
+  KuduSchemaBuilder builder;
+  KuduSchema schema;
+  builder.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey();
+  builder.AddColumn("value")->Type(KuduColumnSchema::INT32)->NotNull();
+  ASSERT_OK(builder.Build(&schema));
+
+  unique_ptr<KuduPartialRow> lower_bound(schema.NewRow());
+  ASSERT_OK(lower_bound->SetInt32("key", 10));
+  unique_ptr<KuduPartialRow> upper_bound(schema.NewRow());
+  ASSERT_OK(upper_bound->SetInt32("key", 10));
+
+  unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
+  table_creator->add_range_partition(lower_bound.release(), upper_bound.release(),
+                                     KuduTableCreator::EXCLUSIVE_BOUND,
+                                     KuduTableCreator::INCLUSIVE_BOUND);
+
+  Status s = table_creator->table_name("TestEqualRangeBounds")
+      .schema(&schema)
+      .num_replicas(1)
+      .set_range_partition_columns({ "key" })
+      .Create();
+
+  ASSERT_TRUE(s.IsInvalidArgument());
+  ASSERT_STR_CONTAINS(s.ToString(),
+                      "Error creating table TestEqualRangeBounds on the master: "
+                          "range partition lower bound must be less than the upper bound");
+}
+
+TEST_F(ClientTest, TestMinMaxRangeBounds) {
+  KuduSchemaBuilder builder;
+  KuduSchema schema;
+  builder.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey();
+  builder.AddColumn("value")->Type(KuduColumnSchema::INT32)->NotNull();
+  ASSERT_OK(builder.Build(&schema));
+
+  unique_ptr<KuduPartialRow> lower_bound(schema.NewRow());
+  ASSERT_OK(lower_bound->SetInt32("key", INT32_MIN));
+  unique_ptr<KuduPartialRow> upper_bound(schema.NewRow());
+  ASSERT_OK(upper_bound->SetInt32("key", INT32_MAX));
+
+  unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
+  table_creator->add_range_partition(lower_bound.release(), upper_bound.release(),
+                                     KuduTableCreator::EXCLUSIVE_BOUND,
+                                     KuduTableCreator::INCLUSIVE_BOUND);
+
+  ASSERT_OK(table_creator->table_name("TestMinMaxRangeBounds")
+      .schema(&schema)
+      .num_replicas(1)
+      .set_range_partition_columns({ "key" })
+      .Create());
+}
+
 TEST_F(ClientTest, TestMetaCacheExpiry) {
   google::FlagSaver saver;
   FLAGS_table_locations_ttl_ms = 25;

http://git-wip-us.apache.org/repos/asf/kudu/blob/0815a123/src/kudu/common/partition.cc
----------------------------------------------------------------------
diff --git a/src/kudu/common/partition.cc b/src/kudu/common/partition.cc
index 2132f26..0fd8850 100644
--- a/src/kudu/common/partition.cc
+++ b/src/kudu/common/partition.cc
@@ -38,7 +38,9 @@
 #include "kudu/gutil/strings/join.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/util/bitmap.h"
+#include "kudu/util/decimal_util.h"
 #include "kudu/util/hash_util.h"
+#include "kudu/util/int128.h"
 #include "kudu/util/logging.h"
 #include "kudu/util/memory/arena.h"
 #include "kudu/util/pb_util.h"
@@ -1000,7 +1002,8 @@ namespace {
   // Increments an unset column in the row.
   Status IncrementUnsetColumn(KuduPartialRow* row, int32_t idx) {
     DCHECK(!row->IsColumnSet(idx));
-    switch (row->schema()->column(idx).type_info()->type()) {
+    const ColumnSchema& col = row->schema()->column(idx);
+    switch (col.type_info()->type()) {
       case INT8:
         RETURN_NOT_OK(row->SetInt8(idx, INT8_MIN + 1));
         break;
@@ -1020,6 +1023,12 @@ namespace {
       case BINARY:
         RETURN_NOT_OK(row->SetBinaryCopy(idx, Slice("\0", 1)));
         break;
+      case DECIMAL32:
+      case DECIMAL64:
+      case DECIMAL128:
+        RETURN_NOT_OK(row->SetUnscaledDecimal(idx,
+            MinUnscaledDecimal(col.type_attributes().precision) + 1));
+        break;
       default:
         return Status::InvalidArgument("Invalid column type in range partition",
                                        row->schema()->column(idx).ToString());
@@ -1031,8 +1040,9 @@ namespace {
   // succeeds, or false if the column is already the maximum value.
   Status IncrementColumn(KuduPartialRow* row, int32_t idx, bool* success) {
     DCHECK(row->IsColumnSet(idx));
+    const ColumnSchema& col = row->schema()->column(idx);
     *success = true;
-    switch (row->schema()->column(idx).type_info()->type()) {
+    switch (col.type_info()->type()) {
       case INT8: {
         int8_t value;
         RETURN_NOT_OK(row->GetInt8(idx, &value));
@@ -1083,6 +1093,18 @@ namespace {
         }
         break;
       }
+      case DECIMAL32:
+      case DECIMAL64:
+      case DECIMAL128: {
+        int128_t value;
+        RETURN_NOT_OK(row->GetUnscaledDecimal(idx, &value));
+        if (value < MaxUnscaledDecimal(col.type_attributes().precision)) {
+          RETURN_NOT_OK(row->SetUnscaledDecimal(idx, value + 1));
+        } else {
+          *success = false;
+        }
+        break;
+      }
       case BINARY: {
         Slice value;
         RETURN_NOT_OK(row->GetBinary(idx, &value));

http://git-wip-us.apache.org/repos/asf/kudu/blob/0815a123/src/kudu/util/decimal_util-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/decimal_util-test.cc b/src/kudu/util/decimal_util-test.cc
index e8c9fc1..d7bfc35 100644
--- a/src/kudu/util/decimal_util-test.cc
+++ b/src/kudu/util/decimal_util-test.cc
@@ -33,6 +33,14 @@ TEST(TestDecimalUtil, TestMaxUnscaledDecimal) {
   ASSERT_EQ(kMaxUnscaledDecimal128, MaxUnscaledDecimal(kMaxDecimal128Precision));
 }
 
+TEST(TestDecimalUtil, TestMinUnscaledDecimal) {
+  ASSERT_EQ(-9, MinUnscaledDecimal(1));
+  ASSERT_EQ(-99999, MinUnscaledDecimal(5));
+  ASSERT_EQ(kMinUnscaledDecimal32, MinUnscaledDecimal(kMaxDecimal32Precision));
+  ASSERT_EQ(kMinUnscaledDecimal64, MinUnscaledDecimal(kMaxDecimal64Precision));
+  ASSERT_EQ(kMinUnscaledDecimal128, MinUnscaledDecimal(kMaxDecimal128Precision));
+}
+
 TEST(TestDecimalUtil, TestToString) {
   ASSERT_EQ("999999999",
             DecimalToString(kMaxUnscaledDecimal32, kDefaultDecimalScale));

http://git-wip-us.apache.org/repos/asf/kudu/blob/0815a123/src/kudu/util/decimal_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/decimal_util.cc b/src/kudu/util/decimal_util.cc
index af81ae5..d4f7e70 100644
--- a/src/kudu/util/decimal_util.cc
+++ b/src/kudu/util/decimal_util.cc
@@ -38,6 +38,10 @@ int128_t MaxUnscaledDecimal(int8_t precision) {
   return result - 1;
 }
 
+int128_t MinUnscaledDecimal(int8_t precision) {
+  return -MaxUnscaledDecimal(precision);
+}
+
 // Workaround for an ASAN build issue documented here:
 // https://bugs.llvm.org/show_bug.cgi?id=16404
 __attribute__((no_sanitize("undefined")))

http://git-wip-us.apache.org/repos/asf/kudu/blob/0815a123/src/kudu/util/decimal_util.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/decimal_util.h b/src/kudu/util/decimal_util.h
index 271bc0e..a465412 100644
--- a/src/kudu/util/decimal_util.h
+++ b/src/kudu/util/decimal_util.h
@@ -57,9 +57,13 @@ namespace kudu {
   // The maximum scale is the Decimal's precision.
 
   // Returns the maximum unscaled decimal value that can be stored
-  // based on the precision
+  // based on the precision.
   int128_t MaxUnscaledDecimal(int8_t precision);
 
+  // Returns the maximum unscaled decimal value that can be stored
+  // based on the precision.
+  int128_t MinUnscaledDecimal(int8_t precision);
+
   std::string DecimalToString(int128_t value, int8_t scale);
 
 } // namespace kudu