You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ad...@apache.org on 2019/03/29 04:18:48 UTC
[kudu] 03/03: KUDU-1711: ColumnSchema supports storing column
comment
This is an automated email from the ASF dual-hosted git repository.
adar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 62d1dec5d5a5c31647d3a92ff203e103198a92d1
Author: helifu <hz...@corp.netease.com>
AuthorDate: Tue Mar 26 10:51:05 2019 +0800
KUDU-1711: ColumnSchema supports storing column comment
This patch intends to support storing column comment in ColumnSchema,
and the comment is displayed on the 'master:port/table?id=uuid' and
'tserver:port/tablet?id=uuid'.
Note: in this patch c++ client has been added 'Comment()' API, but
other clients are not yet.
Change-Id: I82d3228100c262cc4a5c63eaf696927c88bc8990
Reviewed-on: http://gerrit.cloudera.org:8080/12849
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
src/kudu/client/batcher.cc | 4 +-
src/kudu/client/client-test.cc | 112 ++++++++++++++++++++++++++++++
src/kudu/client/schema-internal.h | 4 ++
src/kudu/client/schema.cc | 20 ++++--
src/kudu/client/schema.h | 18 ++++-
src/kudu/client/table_alterer-internal.cc | 10 ++-
src/kudu/common/common.proto | 5 ++
src/kudu/common/schema-test.cc | 6 +-
src/kudu/common/schema.cc | 3 +
src/kudu/common/schema.h | 33 +++++++--
src/kudu/common/wire_protocol.cc | 18 ++++-
src/kudu/common/wire_protocol.h | 2 +
src/kudu/master/catalog_manager.cc | 37 +++++++---
src/kudu/server/webui_util.cc | 3 +
www/table.mustache | 2 +
www/tablet.mustache | 2 +
16 files changed, 253 insertions(+), 26 deletions(-)
diff --git a/src/kudu/client/batcher.cc b/src/kudu/client/batcher.cc
index 716a1ec..e6ce9dd 100644
--- a/src/kudu/client/batcher.cc
+++ b/src/kudu/client/batcher.cc
@@ -304,7 +304,9 @@ WriteRpc::WriteRpc(const scoped_refptr<Batcher>& batcher,
// Set up schema
CHECK_OK(SchemaToPB(*schema, req_.mutable_schema(),
- SCHEMA_PB_WITHOUT_STORAGE_ATTRIBUTES | SCHEMA_PB_WITHOUT_IDS));
+ SCHEMA_PB_WITHOUT_STORAGE_ATTRIBUTES |
+ SCHEMA_PB_WITHOUT_IDS |
+ SCHEMA_PB_WITHOUT_COMMENT));
// Pick up the authz token for the table.
FetchCachedAuthzToken();
diff --git a/src/kudu/client/client-test.cc b/src/kudu/client/client-test.cc
index 44d6f2a..a75fd6d 100644
--- a/src/kudu/client/client-test.cc
+++ b/src/kudu/client/client-test.cc
@@ -127,6 +127,7 @@ DECLARE_int32(leader_failure_exp_backoff_max_delta_ms);
DECLARE_int32(log_inject_latency_ms_mean);
DECLARE_int32(log_inject_latency_ms_stddev);
DECLARE_int32(master_inject_latency_on_tablet_lookups_ms);
+DECLARE_int32(max_column_comment_length);
DECLARE_int32(max_create_tablets_per_ts);
DECLARE_int32(raft_heartbeat_interval_ms);
DECLARE_int32(scanner_batch_size_rows);
@@ -4886,6 +4887,117 @@ TEST_F(ClientTest, TestReadAtSnapshotNoTimestampSet) {
EXPECT_EQ(kTabletsNum * kRowsPerTablet, total_row_count);
}
+TEST_F(ClientTest, TestCreateTableWithValidComment) {
+ const string kTableName = "table_comment";
+ const string kLongComment(FLAGS_max_column_comment_length, 'x');
+
+ // Create a table with comment.
+ {
+ KuduSchema schema;
+ KuduSchemaBuilder schema_builder;
+ schema_builder.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey();
+ schema_builder.AddColumn("val")->Type(KuduColumnSchema::INT32)->Comment(kLongComment);
+ ASSERT_OK(schema_builder.Build(&schema));
+ unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
+ ASSERT_OK(table_creator->table_name(kTableName)
+ .schema(&schema).num_replicas(1).set_range_partition_columns({ "key" }).Create());
+ }
+
+ // Open the table and verify the comment.
+ {
+ KuduSchema schema;
+ ASSERT_OK(client_->GetTableSchema(kTableName, &schema));
+ ASSERT_EQ(schema.Column(1).name(), "val");
+ ASSERT_EQ(schema.Column(1).comment(), kLongComment);
+ }
+}
+
+TEST_F(ClientTest, TestAlterTableWithValidComment) {
+ const string kTableName = "table_comment";
+ const auto AlterAndVerify = [&] (int i) {
+ // The comment length should be less and equal than FLAGS_max_column_comment_length.
+ string kLongComment(i * 16, 'x');
+
+ // Alter the table with comment.
+ unique_ptr<KuduTableAlterer> table_alterer(client_->NewTableAlterer(kTableName));
+ table_alterer->AlterColumn("val")->Comment(kLongComment);
+ ASSERT_OK(table_alterer->Alter());
+
+ // Open the table and verify the comment.
+ KuduSchema schema;
+ ASSERT_OK(client_->GetTableSchema(kTableName, &schema));
+ ASSERT_EQ(schema.Column(1).name(), "val");
+ ASSERT_EQ(schema.Column(1).comment(), kLongComment);
+ };
+
+ // Create a table.
+ {
+ KuduSchema schema;
+ KuduSchemaBuilder schema_builder;
+ schema_builder.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey();
+ schema_builder.AddColumn("val")->Type(KuduColumnSchema::INT32);
+ ASSERT_OK(schema_builder.Build(&schema));
+ unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
+ ASSERT_OK(table_creator->table_name(kTableName)
+ .schema(&schema).num_replicas(1).set_range_partition_columns({ "key" }).Create());
+ }
+
+ for (int i = 0; i <= 16; ++i) {
+ NO_FATALS(AlterAndVerify(i));
+ }
+}
+
+TEST_F(ClientTest, TestCreateAndAlterTableWithInvalidComment) {
+ const string kTableName = "table_comment";
+ const vector<pair<string, string>> kCases = {
+ {string(FLAGS_max_column_comment_length + 1, 'x'), "longer than maximum permitted length"},
+ {string("foo\0bar", 7), "invalid column comment: identifier must not contain null bytes"},
+ {string("foo\xf0\x28\x8c\xbc", 7), "invalid column comment: invalid UTF8 sequence"}
+ };
+
+ // Create tables with invalid comment.
+ for (const auto& test_case : kCases) {
+ const auto& comment = test_case.first;
+ const auto& substr = test_case.second;
+ SCOPED_TRACE(comment);
+
+ KuduSchema schema;
+ KuduSchemaBuilder schema_builder;
+ schema_builder.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey();
+ schema_builder.AddColumn("val")->Type(KuduColumnSchema::INT32)->Comment(comment);
+ ASSERT_OK(schema_builder.Build(&schema));
+ unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
+ Status s = table_creator->table_name(kTableName)
+ .schema(&schema).num_replicas(1).set_range_partition_columns({ "key" }).Create();
+ ASSERT_STR_CONTAINS(s.ToString(), substr);
+ }
+
+ // Create a table for later alterer.
+ {
+ KuduSchema schema;
+ KuduSchemaBuilder schema_builder;
+ schema_builder.AddColumn("key")->Type(KuduColumnSchema::INT32)->NotNull()->PrimaryKey();
+ schema_builder.AddColumn("val")->Type(KuduColumnSchema::INT32);
+ ASSERT_OK(schema_builder.Build(&schema));
+ unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
+ ASSERT_OK(table_creator->table_name(kTableName)
+ .schema(&schema).num_replicas(1).set_range_partition_columns({ "key" }).Create());
+ }
+
+ // Alter tables with invalid comment.
+ for (const auto& test_case : kCases) {
+ const auto& comment = test_case.first;
+ const auto& substr = test_case.second;
+ SCOPED_TRACE(comment);
+
+ // Alter the table.
+ unique_ptr<KuduTableAlterer> table_alterer(client_->NewTableAlterer(kTableName));
+ table_alterer->AlterColumn("val")->Comment(comment);
+ Status s = table_alterer->Alter();
+ ASSERT_STR_CONTAINS(s.ToString(), substr);
+ }
+}
+
enum IntEncoding {
kPlain,
kBitShuffle,
diff --git a/src/kudu/client/schema-internal.h b/src/kudu/client/schema-internal.h
index 10f6132..923ea35 100644
--- a/src/kudu/client/schema-internal.h
+++ b/src/kudu/client/schema-internal.h
@@ -19,6 +19,8 @@
#include <string>
+#include <boost/optional/optional.hpp>
+
#include "kudu/client/schema.h"
#include "kudu/client/value.h"
#include "kudu/common/common.pb.h"
@@ -112,6 +114,8 @@ class KuduColumnSpec::Data {
// For ALTER
bool has_rename_to;
std::string rename_to;
+
+ boost::optional<std::string> comment;
};
} // namespace client
diff --git a/src/kudu/client/schema.cc b/src/kudu/client/schema.cc
index 3ce5a1b..33bbab7 100644
--- a/src/kudu/client/schema.cc
+++ b/src/kudu/client/schema.cc
@@ -289,6 +289,11 @@ KuduColumnSpec* KuduColumnSpec::RenameTo(const std::string& new_name) {
return this;
}
+KuduColumnSpec* KuduColumnSpec::Comment(const string& comment) {
+ data_->comment = boost::optional<string>(comment);
+ return this;
+}
+
Status KuduColumnSpec::ToColumnSchema(KuduColumnSchema* col) const {
// Verify that the user isn't trying to use any methods that
// don't make sense for CREATE.
@@ -377,7 +382,7 @@ Status KuduColumnSpec::ToColumnSchema(KuduColumnSchema* col) const {
*col = KuduColumnSchema(data_->name, data_->type, nullable,
default_val,
KuduColumnStorageAttributes(encoding, compression, block_size),
- type_attrs);
+ type_attrs, data_->comment.get_ptr());
#pragma GCC diagnostic pop
return Status::OK();
@@ -424,6 +429,7 @@ Status KuduColumnSpec::ToColumnSchemaDelta(ColumnSchemaDelta* col_delta) const {
col_delta->cfile_block_size = boost::optional<int32_t>(data_->block_size);
}
+ col_delta->new_comment = std::move(data_->comment);
return Status::OK();
}
@@ -602,7 +608,8 @@ KuduColumnSchema::KuduColumnSchema(const std::string &name,
bool is_nullable,
const void* default_value,
const KuduColumnStorageAttributes& storage_attributes,
- const KuduColumnTypeAttributes& type_attributes) {
+ const KuduColumnTypeAttributes& type_attributes,
+ const string* comment) {
ColumnStorageAttributes attr_private;
attr_private.encoding = ToInternalEncodingType(storage_attributes.encoding());
attr_private.compression = ToInternalCompressionType(
@@ -613,7 +620,8 @@ KuduColumnSchema::KuduColumnSchema(const std::string &name,
col_ = new ColumnSchema(name, ToInternalDataType(type, type_attributes),
is_nullable,
default_value, default_value, attr_private,
- type_attr_private);
+ type_attr_private,
+ std::move(comment ? boost::optional<string>(*comment) : boost::none));
}
KuduColumnSchema::KuduColumnSchema(const KuduColumnSchema& other)
@@ -667,6 +675,10 @@ KuduColumnTypeAttributes KuduColumnSchema::type_attributes() const {
return KuduColumnTypeAttributes(type_attributes.precision, type_attributes.scale);
}
+string KuduColumnSchema::comment() const {
+ return DCHECK_NOTNULL(col_)->comment() ? *col_->comment() : "";
+}
+
////////////////////////////////////////////////////////////
// KuduSchema
////////////////////////////////////////////////////////////
@@ -728,7 +740,7 @@ KuduColumnSchema KuduSchema::Column(size_t idx) const {
KuduColumnTypeAttributes type_attrs(col.type_attributes().precision, col.type_attributes().scale);
return KuduColumnSchema(col.name(), FromInternalDataType(col.type_info()->type()),
col.is_nullable(), col.read_default_value(),
- attrs, type_attrs);
+ attrs, type_attrs, col.comment().get_ptr());
}
KuduPartialRow* KuduSchema::NewRow() const {
diff --git a/src/kudu/client/schema.h b/src/kudu/client/schema.h
index 5495ab7..b59fa39 100644
--- a/src/kudu/client/schema.h
+++ b/src/kudu/client/schema.h
@@ -28,6 +28,7 @@
#ifdef KUDU_HEADERS_NO_STUBS
#include <gtest/gtest_prod.h>
+
#include "kudu/gutil/port.h"
#else
#include "kudu/client/stubs.h"
@@ -241,6 +242,12 @@ class KUDU_EXPORT KuduColumnSchema {
/// @return Type attributes of the column schema.
KuduColumnTypeAttributes type_attributes() const;
+ /// @return comment of the column schema.
+ ///
+ /// @note Both columns with no comments and empty comments will return
+ /// empty strings here.
+ std::string comment() const;
+
private:
friend class KuduColumnSpec;
friend class KuduSchema;
@@ -267,7 +274,9 @@ class KUDU_EXPORT KuduColumnSchema {
bool is_nullable = false,
const void* default_value = NULL, //NOLINT(modernize-use-nullptr)
const KuduColumnStorageAttributes& storage_attributes = KuduColumnStorageAttributes(),
- const KuduColumnTypeAttributes& type_attributes = KuduColumnTypeAttributes());
+ const KuduColumnTypeAttributes& type_attributes = KuduColumnTypeAttributes(),
+ const std::string* comment = NULL //NOLINT(modernize-use-nullptr)
+ );
#if defined(__clang__) || \
(defined(__GNUC__) && (__GNUC__ * 10000 + __GNUC_MINOR__ * 100) >= 40600)
#pragma GCC diagnostic pop
@@ -432,6 +441,13 @@ class KUDU_EXPORT KuduColumnSpec {
KuduColumnSpec* RenameTo(const std::string& new_name);
///@}
+ /// Set the comment of the column.
+ ///
+ /// @param [in] comment
+ /// The comment for the column.
+ /// @return Pointer to the modified object.
+ KuduColumnSpec* Comment(const std::string& comment);
+
private:
class KUDU_NO_EXPORT Data;
friend class KuduSchemaBuilder;
diff --git a/src/kudu/client/table_alterer-internal.cc b/src/kudu/client/table_alterer-internal.cc
index 650d0e0..4172ec3 100644
--- a/src/kudu/client/table_alterer-internal.cc
+++ b/src/kudu/client/table_alterer-internal.cc
@@ -69,7 +69,9 @@ Status KuduTableAlterer::Data::ToRequest(AlterTableRequestPB* req) {
if (schema_ != nullptr) {
RETURN_NOT_OK(SchemaToPB(*schema_, req->mutable_schema(),
- SCHEMA_PB_WITHOUT_IDS | SCHEMA_PB_WITHOUT_WRITE_DEFAULT));
+ SCHEMA_PB_WITHOUT_IDS |
+ SCHEMA_PB_WITHOUT_WRITE_DEFAULT |
+ SCHEMA_PB_WITHOUT_COMMENT));
}
for (const Step& s : steps_) {
@@ -105,7 +107,8 @@ Status KuduTableAlterer::Data::ToRequest(AlterTableRequestPB* req) {
!s.spec->data_->remove_default &&
!s.spec->data_->has_encoding &&
!s.spec->data_->has_compression &&
- !s.spec->data_->has_block_size) {
+ !s.spec->data_->has_block_size &&
+ !s.spec->data_->comment) {
return Status::InvalidArgument("no alter operation specified",
s.spec->data_->name);
}
@@ -117,7 +120,8 @@ Status KuduTableAlterer::Data::ToRequest(AlterTableRequestPB* req) {
!s.spec->data_->remove_default &&
!s.spec->data_->has_encoding &&
!s.spec->data_->has_compression &&
- !s.spec->data_->has_block_size) {
+ !s.spec->data_->has_block_size &&
+ !s.spec->data_->comment) {
pb_step->set_type(AlterTableRequestPB::RENAME_COLUMN);
pb_step->mutable_rename_column()->set_old_name(s.spec->data_->name);
pb_step->mutable_rename_column()->set_new_name(s.spec->data_->rename_to);
diff --git a/src/kudu/common/common.proto b/src/kudu/common/common.proto
index ceb281d..a51e806 100644
--- a/src/kudu/common/common.proto
+++ b/src/kudu/common/common.proto
@@ -116,6 +116,9 @@ message ColumnSchemaPB {
optional int32 cfile_block_size = 10 [default=0];
optional ColumnTypeAttributesPB type_attributes = 11;
+
+ // The comment for the column.
+ optional string comment = 12;
}
message ColumnSchemaDeltaPB {
@@ -128,6 +131,8 @@ message ColumnSchemaDeltaPB {
optional EncodingType encoding = 6;
optional CompressionType compression = 7;
optional int32 block_size = 8;
+
+ optional string new_comment = 9;
}
message SchemaPB {
diff --git a/src/kudu/common/schema-test.cc b/src/kudu/common/schema-test.cc
index ee428f7..1c5e8c6 100644
--- a/src/kudu/common/schema-test.cc
+++ b/src/kudu/common/schema-test.cc
@@ -273,9 +273,9 @@ TEST_F(TestSchema, TestColumnSchemaEquals) {
ASSERT_TRUE(col1.Equals(col2, ColumnSchema::COMPARE_TYPE));
ASSERT_TRUE(col1.Equals(col3, ColumnSchema::COMPARE_NAME));
ASSERT_FALSE(col1.Equals(col3, ColumnSchema::COMPARE_TYPE));
- ASSERT_TRUE(col1.Equals(col3, ColumnSchema::COMPARE_DEFAULTS));
- ASSERT_FALSE(col3.Equals(col4, ColumnSchema::COMPARE_DEFAULTS));
- ASSERT_TRUE(col4.Equals(col4, ColumnSchema::COMPARE_DEFAULTS));
+ ASSERT_TRUE(col1.Equals(col3, ColumnSchema::COMPARE_OTHER));
+ ASSERT_FALSE(col3.Equals(col4, ColumnSchema::COMPARE_OTHER));
+ ASSERT_TRUE(col4.Equals(col4, ColumnSchema::COMPARE_OTHER));
}
TEST_F(TestSchema, TestSchemaEquals) {
diff --git a/src/kudu/common/schema.cc b/src/kudu/common/schema.cc
index 3cb3c98..81ba0a2 100644
--- a/src/kudu/common/schema.cc
+++ b/src/kudu/common/schema.cc
@@ -115,6 +115,9 @@ Status ColumnSchema::ApplyDelta(const ColumnSchemaDelta& col_delta) {
if (col_delta.cfile_block_size) {
attributes_.cfile_block_size = *col_delta.cfile_block_size;
}
+ if (col_delta.new_comment) {
+ comment_ = col_delta.new_comment;
+ }
return Status::OK();
}
diff --git a/src/kudu/common/schema.h b/src/kudu/common/schema.h
index bc4cefd..89dbdd9 100644
--- a/src/kudu/common/schema.h
+++ b/src/kudu/common/schema.h
@@ -174,6 +174,8 @@ public:
boost::optional<EncodingType> encoding;
boost::optional<CompressionType> compression;
boost::optional<int32_t> cfile_block_size;
+
+ boost::optional<std::string> new_comment;
};
// The schema for a given column.
@@ -190,6 +192,7 @@ class ColumnSchema {
// write_default: default value added to the row if the column value was
// not specified on insert.
// The value will be copied and released on ColumnSchema destruction.
+ // comment: the comment for the column.
//
// Example:
// ColumnSchema col_a("a", UINT32)
@@ -202,13 +205,15 @@ class ColumnSchema {
const void* read_default = nullptr,
const void* write_default = nullptr,
ColumnStorageAttributes attributes = ColumnStorageAttributes(),
- ColumnTypeAttributes type_attributes = ColumnTypeAttributes())
+ ColumnTypeAttributes type_attributes = ColumnTypeAttributes(),
+ boost::optional<std::string> comment = boost::none)
: name_(std::move(name)),
type_info_(GetTypeInfo(type)),
is_nullable_(is_nullable),
read_default_(read_default ? new Variant(type, read_default) : nullptr),
attributes_(attributes),
- type_attributes_(type_attributes) {
+ type_attributes_(type_attributes),
+ comment_(std::move(comment)) {
if (write_default == read_default) {
write_default_ = read_default_;
} else if (write_default != nullptr) {
@@ -248,6 +253,10 @@ class ColumnSchema {
// For example, "AUTO_ENCODING ZLIB 123 123".
std::string AttrToString() const;
+ const boost::optional<std::string>& comment() const {
+ return comment_;
+ }
+
// Returns true if the column has a read default value
bool has_read_default() const {
return read_default_ != nullptr;
@@ -303,10 +312,10 @@ class ColumnSchema {
enum CompareFlags {
COMPARE_NAME = 1 << 0,
COMPARE_TYPE = 1 << 1,
- COMPARE_DEFAULTS = 1 << 2,
+ COMPARE_OTHER = 1 << 2,
COMPARE_NAME_AND_TYPE = COMPARE_NAME | COMPARE_TYPE,
- COMPARE_ALL = COMPARE_NAME | COMPARE_TYPE | COMPARE_DEFAULTS
+ COMPARE_ALL = COMPARE_NAME | COMPARE_TYPE | COMPARE_OTHER
};
bool Equals(const ColumnSchema &other,
@@ -322,7 +331,7 @@ class ColumnSchema {
// For Key comparison checking the defaults doesn't make sense,
// since we don't support them, for server vs user schema this comparison
// will always fail, since the user does not specify the defaults.
- if (flags & COMPARE_DEFAULTS) {
+ if (flags & COMPARE_OTHER) {
if (read_default_ == nullptr && other.read_default_ != nullptr)
return false;
@@ -334,6 +343,19 @@ class ColumnSchema {
if (write_default_ != nullptr && !write_default_->Equals(other.write_default_.get()))
return false;
+
+ // "no comment" and "empty comment" are the same.
+ if (comment_) {
+ if (other.comment_) {
+ if (*comment_ != *other.comment_) return false;
+ } else {
+ if (!comment_->empty()) return false;
+ }
+ } else {
+ if (other.comment_) {
+ if (!other.comment_->empty()) return false;
+ }
+ }
}
return true;
}
@@ -405,6 +427,7 @@ class ColumnSchema {
std::shared_ptr<Variant> write_default_;
ColumnStorageAttributes attributes_;
ColumnTypeAttributes type_attributes_;
+ boost::optional<std::string> comment_;
};
// The schema for a set of rows.
diff --git a/src/kudu/common/wire_protocol.cc b/src/kudu/common/wire_protocol.cc
index dc7b1f2..fa30e15 100644
--- a/src/kudu/common/wire_protocol.cc
+++ b/src/kudu/common/wire_protocol.cc
@@ -23,6 +23,7 @@
#include <cstring>
#include <ostream>
#include <string>
+#include <utility>
#include <vector>
#include <boost/optional/optional.hpp>
@@ -246,6 +247,9 @@ void ColumnSchemaToPB(const ColumnSchema& col_schema, ColumnSchemaPB *pb, int fl
pb->set_write_default_value(write_value, col_schema.type_info()->size());
}
}
+ if (col_schema.comment() && !(flags & SCHEMA_PB_WITHOUT_COMMENT)) {
+ pb->set_comment(*col_schema.comment());
+ }
}
ColumnSchema ColumnSchemaFromPB(const ColumnSchemaPB& pb) {
@@ -292,9 +296,15 @@ ColumnSchema ColumnSchemaFromPB(const ColumnSchemaPB& pb) {
if (pb.has_cfile_block_size()) {
attributes.cfile_block_size = pb.cfile_block_size();
}
+
+ boost::optional<string> comment;
+ if (pb.has_comment()) {
+ comment = boost::optional<string>(pb.comment());
+ }
+
return ColumnSchema(pb.name(), pb.type(), pb.is_nullable(),
read_default_ptr, write_default_ptr,
- attributes, type_attributes);
+ attributes, type_attributes, std::move(comment));
}
void ColumnSchemaDeltaToPB(const ColumnSchemaDelta& col_delta, ColumnSchemaDeltaPB *pb) {
@@ -319,6 +329,9 @@ void ColumnSchemaDeltaToPB(const ColumnSchemaDelta& col_delta, ColumnSchemaDelta
if (col_delta.cfile_block_size) {
pb->set_block_size(*col_delta.cfile_block_size);
}
+ if (col_delta.new_comment) {
+ pb->set_new_comment(*col_delta.new_comment);
+ }
}
ColumnSchemaDelta ColumnSchemaDeltaFromPB(const ColumnSchemaDeltaPB& pb) {
@@ -341,6 +354,9 @@ ColumnSchemaDelta ColumnSchemaDeltaFromPB(const ColumnSchemaDeltaPB& pb) {
if (pb.has_block_size()) {
col_delta.cfile_block_size = boost::optional<int32_t>(pb.block_size());
}
+ if (pb.has_new_comment()) {
+ col_delta.new_comment = boost::optional<string>(pb.new_comment());
+ }
return col_delta;
}
diff --git a/src/kudu/common/wire_protocol.h b/src/kudu/common/wire_protocol.h
index cf6be92..3cb1883 100644
--- a/src/kudu/common/wire_protocol.h
+++ b/src/kudu/common/wire_protocol.h
@@ -92,6 +92,8 @@ enum SchemaPBConversionFlags {
// protobuf. Used when sending schemas from the client to the master
// for create/alter table.
SCHEMA_PB_WITHOUT_WRITE_DEFAULT = 1 << 2,
+
+ SCHEMA_PB_WITHOUT_COMMENT = 1 << 3,
};
// Convert the specified schema to protobuf.
diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
index ed5d583..3a2f9b2 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -180,6 +180,10 @@ TAG_FLAG(max_num_columns, unsafe);
DEFINE_int32(max_identifier_length, 256,
"Maximum length of the name of a column or table.");
+
+DEFINE_int32(max_column_comment_length, 256,
+ "Maximum length of the comment of a column");
+
// Tag as unsafe because we end up writing schemas in every WAL entry, etc,
// and having very long column names would enter untested territory and affect
// performance.
@@ -1297,19 +1301,15 @@ Status CatalogManager::CheckOnline() const {
namespace {
-// Validate a table or column name to ensure that it is a valid identifier.
-Status ValidateIdentifier(const string& id) {
- if (id.empty()) {
- return Status::InvalidArgument("empty string not a valid identifier");
- }
-
- if (id.length() > FLAGS_max_identifier_length) {
+Status ValidateLengthAndUTF8(const string& id, int32_t max_length) {
+ // Id should not exceed the maximum allowed length.
+ if (id.length() > max_length) {
return Status::InvalidArgument(Substitute(
"identifier '$0' longer than maximum permitted length $1",
id, FLAGS_max_identifier_length));
}
- // Identifiers should be valid UTF8.
+ // Id should be valid UTF8.
const char* p = id.data();
int rem = id.size();
while (rem > 0) {
@@ -1328,6 +1328,25 @@ Status ValidateIdentifier(const string& id) {
return Status::OK();
}
+// Validate a table or column name to ensure that it is a valid identifier.
+Status ValidateIdentifier(const string& id) {
+ if (id.empty()) {
+ return Status::InvalidArgument("empty string not a valid identifier");
+ }
+
+ return ValidateLengthAndUTF8(id, FLAGS_max_identifier_length);
+}
+
+// Validate a column comment to ensure that it is a valid identifier.
+Status ValidateCommentIdentifier(const boost::optional<string>& id) {
+ // It's safe here because logical OR has left-to-right associativity.
+ if (!id || id->empty()) {
+ return Status::OK();
+ }
+
+ return ValidateLengthAndUTF8(*id, FLAGS_max_column_comment_length);
+}
+
// Validate the client-provided schema and name.
Status ValidateClientSchema(const optional<string>& name,
const Schema& schema) {
@@ -1337,6 +1356,8 @@ Status ValidateClientSchema(const optional<string>& name,
for (int i = 0; i < schema.num_columns(); i++) {
RETURN_NOT_OK_PREPEND(ValidateIdentifier(schema.column(i).name()),
"invalid column name");
+ RETURN_NOT_OK_PREPEND(ValidateCommentIdentifier(schema.column(i).comment()),
+ "invalid column comment");
}
if (schema.num_key_columns() <= 0) {
return Status::InvalidArgument("must specify at least one key column");
diff --git a/src/kudu/server/webui_util.cc b/src/kudu/server/webui_util.cc
index 590080f..44a45be 100644
--- a/src/kudu/server/webui_util.cc
+++ b/src/kudu/server/webui_util.cc
@@ -19,6 +19,8 @@
#include <string>
+#include <boost/optional/optional.hpp>
+
#include "kudu/common/common.pb.h"
#include "kudu/common/schema.h"
#include "kudu/gutil/ref_counted.h"
@@ -50,6 +52,7 @@ void SchemaToJson(const Schema& schema, EasyJson* output) {
col.Stringify(col.read_default_value()) : "-";
col_json["write_default"] = col.has_write_default() ?
col.Stringify(col.write_default_value()) : "-";
+ col_json["comment"] = col.comment() ? *col.comment() : "-";
}
}
diff --git a/www/table.mustache b/www/table.mustache
index 8af9ebd..6a960d9 100644
--- a/www/table.mustache
+++ b/www/table.mustache
@@ -44,6 +44,7 @@ under the License.
<th>Compression</th>
<th>Read default</th>
<th>Write default</th>
+ <th>Comment</th>
</tr></thead>
<tbody>
{{#columns}}
@@ -57,6 +58,7 @@ under the License.
<td>{{compression}}</td>
<td>{{read_default}}</td>
<td>{{write_default}}</td>
+ <td>{{comment}}</td>
</tr>
{{/columns}}
</tbody>
diff --git a/www/tablet.mustache b/www/tablet.mustache
index aa83558..cbc0ff5 100644
--- a/www/tablet.mustache
+++ b/www/tablet.mustache
@@ -38,6 +38,7 @@ under the License.
<th>Compression</th>
<th>Read default</th>
<th>Write default</th>
+ <th>Comment</th>
</tr></thead>
<tbody>
{{#columns}}
@@ -51,6 +52,7 @@ under the License.
<td>{{compression}}</td>
<td>{{read_default}}</td>
<td>{{write_default}}</td>
+ <td>{{comment}}</td>
</tr>
{{/columns}}
</tbody>