This is an automated email from the ASF dual-hosted git repository. alexey pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
commit bbf82c14e6aedb5adc84ddfbc2b1600e21493b98 Author: Marton Greber <[email protected]> AuthorDate: Thu Aug 4 13:14:24 2022 +0200 KUDU-3379 Add column comments to table describe This patch adds -show_column_comment flag to the 'kudu table describe' CLI tool. If there are column comments specified, this can be used to show them. Schema and ColumnSchema use the ToStringMode enum to control how they are stringified. Previously these were used to signal a single mode of the stringification. Now both are populated through bit shifting. This way the enum can be used as a holder of multiple binary flags. We can check for the presence of a flag through 'bitwise and' operator, and set a given flag through the 'bitwise or' operator. A new test case is added to check whether the column describing flags work, alone and together as well. Change-Id: Id8b751bd821a5f50490caa6f1aaf1fc767de0222 Reviewed-on: http://gerrit.cloudera.org:8080/18903 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin <[email protected]> --- src/kudu/client/schema.cc | 18 +++++++-- src/kudu/common/schema.cc | 20 ++++++---- src/kudu/common/schema.h | 19 +++++---- src/kudu/tools/kudu-admin-test.cc | 80 +++++++++++++++++++++++++++++++++++-- src/kudu/tools/tool_action_table.cc | 1 + 5 files changed, 117 insertions(+), 21 deletions(-) diff --git a/src/kudu/client/schema.cc b/src/kudu/client/schema.cc index 75d3ba16b..5576137bf 100644 --- a/src/kudu/client/schema.cc +++ b/src/kudu/client/schema.cc @@ -47,6 +47,9 @@ DEFINE_bool(show_attributes, false, "Whether to show column attributes, including column encoding type, " "compression type, and default read/write value."); +DEFINE_bool(show_column_comment, false, + "Whether to show column comment."); + MAKE_ENUM_LIMITS(kudu::client::KuduColumnStorageAttributes::EncodingType, kudu::client::KuduColumnStorageAttributes::AUTO_ENCODING, kudu::client::KuduColumnStorageAttributes::RLE); @@ -951,10 +954,17 @@ void KuduSchema::GetPrimaryKeyColumnIndexes(vector<int>* indexes) const { } string KuduSchema::ToString() const { - return schema_ ? schema_->ToString(FLAGS_show_attributes ? - Schema::ToStringMode::WITH_COLUMN_ATTRIBUTES - : Schema::ToStringMode::BASE_INFO) - : "()"; + if (!schema_) { + return "()"; + } + uint8_t mode = Schema::ToStringMode::BASE_INFO; + if (FLAGS_show_attributes) { + mode |= Schema::ToStringMode::WITH_COLUMN_ATTRIBUTES; + } + if (FLAGS_show_column_comment) { + mode |= Schema::ToStringMode::WITH_COLUMN_COMMENTS; + } + return schema_->ToString(mode); } KuduSchema KuduSchema::FromSchema(const Schema& schema) { diff --git a/src/kudu/common/schema.cc b/src/kudu/common/schema.cc index 48b14ae38..7fa54cbfa 100644 --- a/src/kudu/common/schema.cc +++ b/src/kudu/common/schema.cc @@ -156,11 +156,14 @@ Status ColumnSchema::ApplyDelta(const ColumnSchemaDelta& col_delta) { return Status::OK(); } -string ColumnSchema::ToString(ToStringMode mode) const { - return Substitute("$0 $1$2", +string ColumnSchema::ToString(uint8_t mode) const { + return Substitute("$0 $1$2$3", name_, TypeToString(), - mode == ToStringMode::WITH_ATTRIBUTES ? " " + AttrToString() : ""); + mode & ToStringMode::WITH_ATTRIBUTES ? + " " + AttrToString() : "", + mode & ToStringMode::WITH_COMMENTS + && comment_.length() ? " " + comment_ : ""); } string ColumnSchema::TypeToString() const { @@ -456,7 +459,7 @@ Status Schema::GetMappedReadProjection(const Schema& projection, return Status::OK(); } -string Schema::ToString(ToStringMode mode) const { +string Schema::ToString(uint8_t mode) const { if (cols_.empty()) return "()"; vector<string> pk_strs; @@ -465,12 +468,15 @@ string Schema::ToString(ToStringMode mode) const { pk_strs.push_back(cols_[i].name()); } - auto col_mode = ColumnSchema::ToStringMode::WITHOUT_ATTRIBUTES; + uint8_t col_mode = ColumnSchema::ToStringMode::WITHOUT_ATTRIBUTES; if (mode & ToStringMode::WITH_COLUMN_ATTRIBUTES) { - col_mode = ColumnSchema::ToStringMode::WITH_ATTRIBUTES; + col_mode |= ColumnSchema::ToStringMode::WITH_ATTRIBUTES; + } + if (mode & ToStringMode::WITH_COLUMN_COMMENTS) { + col_mode |= ColumnSchema::ToStringMode::WITH_COMMENTS; } vector<string> col_strs; - if (has_column_ids() && (mode & ToStringMode::WITH_COLUMN_IDS)) { + if (has_column_ids() && mode & ToStringMode::WITH_COLUMN_IDS) { for (size_t i = 0; i < cols_.size(); ++i) { col_strs.push_back(Substitute("$0:$1", col_ids_[i], cols_[i].ToString(col_mode))); } diff --git a/src/kudu/common/schema.h b/src/kudu/common/schema.h index df1d725f3..b72bca505 100644 --- a/src/kudu/common/schema.h +++ b/src/kudu/common/schema.h @@ -266,16 +266,18 @@ class ColumnSchema { } // Enum to configure how a ColumnSchema is stringified. - enum class ToStringMode { + enum ToStringMode : uint8_t { + // Do not include below attributes. + WITHOUT_ATTRIBUTES = 0, // Include encoding type, compression type, and default read/write value. - WITH_ATTRIBUTES, - // Do not include above attributes. - WITHOUT_ATTRIBUTES, + WITH_ATTRIBUTES = 1 << 0, + // Include comments + WITH_COMMENTS = 1 << 1 }; // Return a string identifying this column, including its // name. - std::string ToString(ToStringMode mode = ToStringMode::WITHOUT_ATTRIBUTES) const; + std::string ToString(uint8_t mode = ToStringMode::WITHOUT_ATTRIBUTES) const; // Same as above, but only including the type information. // For example, "STRING NOT NULL". @@ -794,16 +796,19 @@ class Schema { } // Enum to configure how a Schema is stringified. - enum ToStringMode { + enum ToStringMode : uint8_t { BASE_INFO = 0, // Include column ids if this instance has them. WITH_COLUMN_IDS = 1 << 0, // Include column attributes. WITH_COLUMN_ATTRIBUTES = 1 << 1, + // Include column comments. + WITH_COLUMN_COMMENTS = 1 << 2 }; + // Stringify this Schema. This is not particularly efficient, // so should only be used when necessary for output. - std::string ToString(ToStringMode mode = ToStringMode::WITH_COLUMN_IDS) const; + std::string ToString(uint8_t mode = ToStringMode::WITH_COLUMN_IDS) const; bool operator==(const Schema& other) const { if (this == &other) { diff --git a/src/kudu/tools/kudu-admin-test.cc b/src/kudu/tools/kudu-admin-test.cc index 215ea1be9..fd8750ae5 100644 --- a/src/kudu/tools/kudu-admin-test.cc +++ b/src/kudu/tools/kudu-admin-test.cc @@ -1914,7 +1914,8 @@ TEST_F(AdminCliTest, TestDescribeTable) { builder.AddColumn("date_val")->Type(KuduColumnSchema::DATE); builder.AddColumn("string_val")->Type(KuduColumnSchema::STRING) ->Encoding(KuduColumnStorageAttributes::EncodingType::PREFIX_ENCODING) - ->Default(KuduValue::CopyString(Slice("hello"))); + ->Default(KuduValue::CopyString(Slice("hello"))) + ->Comment("comment for hello"); builder.AddColumn("bool_val")->Type(KuduColumnSchema::BOOL) ->Default(KuduValue::FromBool(false)); builder.AddColumn("float_val")->Type(KuduColumnSchema::FLOAT); @@ -2002,7 +2003,8 @@ TEST_F(AdminCliTest, TestDescribeTable) { "describe", cluster_->master()->bound_rpc_addr().ToString(), kAnotherTableId, - "-show_attributes=true" + "-show_attributes=true", + "-show_column_comment=true" }, &stdout, &stderr); ASSERT_TRUE(s.ok()) << ToolRunInfo(s, stdout, stderr); @@ -2019,7 +2021,8 @@ TEST_F(AdminCliTest, TestDescribeTable) { " int64_val INT64 NULLABLE AUTO_ENCODING ZLIB 123 123,\n" " timestamp_val UNIXTIME_MICROS NULLABLE AUTO_ENCODING DEFAULT_COMPRESSION - -,\n" " date_val DATE NULLABLE AUTO_ENCODING DEFAULT_COMPRESSION - -,\n" - " string_val STRING NULLABLE PREFIX_ENCODING DEFAULT_COMPRESSION \"hello\" \"hello\",\n" + " string_val STRING NULLABLE PREFIX_ENCODING DEFAULT_COMPRESSION \"hello\" \"hello\" " + "comment for hello,\n" " bool_val BOOL NULLABLE AUTO_ENCODING DEFAULT_COMPRESSION false false,\n" " float_val FLOAT NULLABLE AUTO_ENCODING DEFAULT_COMPRESSION - -,\n" " double_val DOUBLE NULLABLE AUTO_ENCODING DEFAULT_COMPRESSION 123.4 123.4,\n" @@ -2169,6 +2172,77 @@ TEST_F(AdminCliTest, TestDescribeTable) { ); } +// Simple tests to check whether the column describing flags +// work, alone and together as well. +TEST_F(AdminCliTest, TestDescribeTableColumnFlags) { + + NO_FATALS(BuildAndStart()); + string stdout; + const string kTableName = "TestAnotherTable"; + + { + // Build the schema + KuduSchema schema; + KuduSchemaBuilder builder; + builder.AddColumn("foo")->Type(KuduColumnSchema::INT32)->NotNull(); + builder.AddColumn("bar")->Type(KuduColumnSchema::INT32)->NotNull() + ->Comment("comment for bar"); + builder.SetPrimaryKey({"foo"}); + ASSERT_OK(builder.Build(&schema)); + + // Create the table + unique_ptr<KuduTableCreator> table_creator(client_->NewTableCreator()); + ASSERT_OK(table_creator->table_name(kTableName) + .schema(&schema) + .set_range_partition_columns({"foo"}) + .Create()); + } + + // Test the describe output with flags + ASSERT_OK(RunKuduTool({"table", + "describe", + cluster_->master()->bound_rpc_addr().ToString(), + kTableName, + "-show_column_comment"}, + &stdout)); + ASSERT_STR_CONTAINS(stdout, + "(\n" + " foo INT32 NOT NULL,\n" + " bar INT32 NOT NULL comment for bar,\n" + " PRIMARY KEY (foo)\n" + ")\n"); + stdout.clear(); + + ASSERT_OK(RunKuduTool({"table", + "describe", + cluster_->master()->bound_rpc_addr().ToString(), + kTableName, + "-show_attributes"}, + &stdout)); + ASSERT_STR_CONTAINS(stdout, + "(\n" + " foo INT32 NOT NULL AUTO_ENCODING DEFAULT_COMPRESSION - -,\n" + " bar INT32 NOT NULL AUTO_ENCODING DEFAULT_COMPRESSION - -,\n" + " PRIMARY KEY (foo)\n" + ")\n"); + stdout.clear(); + + ASSERT_OK(RunKuduTool({"table", + "describe", + cluster_->master()->bound_rpc_addr().ToString(), + kTableName, + "-show_attributes", + "-show_column_comment"}, + &stdout)); + ASSERT_STR_CONTAINS( + stdout, + "(\n" + " foo INT32 NOT NULL AUTO_ENCODING DEFAULT_COMPRESSION - -,\n" + " bar INT32 NOT NULL AUTO_ENCODING DEFAULT_COMPRESSION - - comment for bar,\n" + " PRIMARY KEY (foo)\n" + ")\n"); +} + TEST_F(AdminCliTest, TestDescribeTableNoOwner) { NO_FATALS(BuildAndStart({}, {"--allow_empty_owner=true"})); KuduSchema schema; diff --git a/src/kudu/tools/tool_action_table.cc b/src/kudu/tools/tool_action_table.cc index 0c578d58a..56608c9f5 100644 --- a/src/kudu/tools/tool_action_table.cc +++ b/src/kudu/tools/tool_action_table.cc @@ -1802,6 +1802,7 @@ unique_ptr<Mode> BuildTableMode() { .Description("Describe a table") .AddRequiredParameter({ kTableNameArg, "Name of the table to describe" }) .AddOptionalParameter("show_attributes") + .AddOptionalParameter("show_column_comment") .AddOptionalParameter("show_avro_format_schema") .Build();
