kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jdcry...@apache.org
Subject [2/3] kudu git commit: KUDU-1599. Reject invalid type/encoding pairs on table creation
Date Tue, 08 Nov 2016 16:54:14 GMT
KUDU-1599. Reject invalid type/encoding pairs on table creation

Change-Id: Ib508dbfbfd574c7c8ccc90ef8c3a41dcbfea058f
Reviewed-on: http://gerrit.cloudera.org:8080/4984
Reviewed-by: Adar Dembo <adar@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/c7090f9c
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/c7090f9c
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/c7090f9c

Branch: refs/heads/master
Commit: c7090f9c00cbaf08ed871d40cd002fa7c70c96de
Parents: f65924d
Author: Todd Lipcon <todd@apache.org>
Authored: Mon Nov 7 17:10:44 2016 -0800
Committer: Todd Lipcon <todd@apache.org>
Committed: Tue Nov 8 02:07:49 2016 +0000

----------------------------------------------------------------------
 src/kudu/cfile/type_encodings.cc   |  2 +-
 src/kudu/client/client-test.cc     | 19 ++++++++++++++++++-
 src/kudu/master/catalog_manager.cc | 26 ++++++++++++++++++++------
 3 files changed, 39 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/c7090f9c/src/kudu/cfile/type_encodings.cc
----------------------------------------------------------------------
diff --git a/src/kudu/cfile/type_encodings.cc b/src/kudu/cfile/type_encodings.cc
index 63bb0ed..dd249ac 100644
--- a/src/kudu/cfile/type_encodings.cc
+++ b/src/kudu/cfile/type_encodings.cc
@@ -238,7 +238,7 @@ class TypeEncodingResolver {
     const TypeEncodingInfo *type_info = mapping_[make_pair(t, e)].get();
     if (PREDICT_FALSE(type_info == nullptr)) {
       return Status::NotSupported(
-          strings::Substitute("Unsupported type/encoding pair: $0, $1",
+          strings::Substitute("encoding $1 not supported for type $0",
                               DataType_Name(t),
                               EncodingType_Name(e)));
     }

http://git-wip-us.apache.org/repos/asf/kudu/blob/c7090f9c/src/kudu/client/client-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client-test.cc b/src/kudu/client/client-test.cc
index 2919280..d7a8a5e 100644
--- a/src/kudu/client/client-test.cc
+++ b/src/kudu/client/client-test.cc
@@ -3033,7 +3033,7 @@ TEST_F(ClientTest, TestBasicAlterOperations) {
       ->Encoding(KuduColumnStorageAttributes::GROUP_VARINT);
     Status s = table_alterer->Alter();
     ASSERT_TRUE(s.IsNotSupported());
-    ASSERT_STR_CONTAINS(s.ToString(), "Unsupported type/encoding pair");
+    ASSERT_STR_CONTAINS(s.ToString(), "encoding GROUP_VARINT not supported for type BINARY");
     ASSERT_EQ(1, tablet_peer->tablet()->metadata()->schema_version());
   }
 
@@ -3637,6 +3637,23 @@ TEST_F(ClientTest, TestCreateTableWithTooManyReplicas) {
                       "replication factor 3. 1 tablet servers are alive");
 }
 
+TEST_F(ClientTest, TestCreateTableWithInvalidEncodings) {
+  unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
+  KuduSchema schema;
+  KuduSchemaBuilder schema_builder;
+  schema_builder.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey()
+      ->Encoding(KuduColumnStorageAttributes::DICT_ENCODING);
+  ASSERT_OK(schema_builder.Build(&schema));
+  Status s = table_creator->table_name("foobar")
+      .schema(&schema)
+      .set_range_partition_columns({ "key" })
+      .Create();
+  ASSERT_TRUE(s.IsNotSupported()) << s.ToString();
+  ASSERT_STR_CONTAINS(s.ToString(),
+                      "invalid encoding for column 'key': encoding "
+                      "DICT_ENCODING not supported for type INT32");
+}
+
 TEST_F(ClientTest, TestLatestObservedTimestamp) {
   // Check that a write updates the latest observed timestamp.
   uint64_t ts0 = client_->GetLatestObservedTimestamp();

http://git-wip-us.apache.org/repos/asf/kudu/blob/c7090f9c/src/kudu/master/catalog_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
index 7730ea6..bdbf767 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -804,10 +804,23 @@ Status CatalogManager::CreateTable(const CreateTableRequestPB* orig_req,
   }
   for (int i = 0; i < client_schema.num_key_columns(); i++) {
     if (!IsTypeAllowableInKey(client_schema.column(i).type_info())) {
-        Status s = Status::InvalidArgument(
-            "Key column may not have type of BOOL, FLOAT, or DOUBLE");
-        SetupError(resp->mutable_error(), MasterErrorPB::INVALID_SCHEMA, s);
-        return s;
+      Status s = Status::InvalidArgument(
+          "Key column may not have type of BOOL, FLOAT, or DOUBLE");
+      SetupError(resp->mutable_error(), MasterErrorPB::INVALID_SCHEMA, s);
+      return s;
+    }
+  }
+  // Check that the encodings are valid for the specified types.
+  for (int i = 0; i < client_schema.num_columns(); i++) {
+    const auto& col = client_schema.column(i);
+    const TypeEncodingInfo *dummy;
+    Status s = TypeEncodingInfo::Get(col.type_info(),
+                                     col.attributes().encoding,
+                                     &dummy);
+    if (!s.ok()) {
+      s = s.CloneAndPrepend(Substitute("invalid encoding for column '$0'", col.name()));
+      SetupError(resp->mutable_error(), MasterErrorPB::INVALID_SCHEMA, s);
+      return s;
     }
   }
   Schema schema = client_schema.CopyWithColumnIds();
@@ -1175,13 +1188,14 @@ Status CatalogManager::ApplyAlterSchemaSteps(const SysTablesEntryPB&
current_pb,
           return Status::InvalidArgument("ADD_COLUMN missing column info");
         }
 
-        // Verify that encoding is appropriate for the new column's
-        // type
         ColumnSchemaPB new_col_pb = step.add_column().schema();
         if (new_col_pb.has_id()) {
           return Status::InvalidArgument("column $0: client should not specify column ID",
                                          new_col_pb.ShortDebugString());
         }
+
+        // Verify that encoding is appropriate for the new column's
+        // type
         ColumnSchema new_col = ColumnSchemaFromPB(new_col_pb);
         const TypeEncodingInfo *dummy;
         RETURN_NOT_OK(TypeEncodingInfo::Get(new_col.type_info(),


Mime
View raw message