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 <alexey@apache.org>
diff --git a/src/kudu/client/schema.cc b/src/kudu/client/schema.cc
index 75d3ba1..5576137 100644
--- a/src/kudu/client/schema.cc
+++ b/src/kudu/client/schema.cc
@@ -47,6 +47,9 @@
             "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 @@
 }
 
 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 48b14ae..7fa54cb 100644
--- a/src/kudu/common/schema.cc
+++ b/src/kudu/common/schema.cc
@@ -156,11 +156,14 @@
   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 @@
   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 @@
     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 df1d725..b72bca5 100644
--- a/src/kudu/common/schema.h
+++ b/src/kudu/common/schema.h
@@ -266,16 +266,18 @@
   }
 
   // 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 @@
   }
 
   // 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 215ea1b..fd8750a 100644
--- a/src/kudu/tools/kudu-admin-test.cc
+++ b/src/kudu/tools/kudu-admin-test.cc
@@ -1914,7 +1914,8 @@
     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 @@
     "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 @@
       "    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 @@
   );
 }
 
+// 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 0c578d5..56608c9 100644
--- a/src/kudu/tools/tool_action_table.cc
+++ b/src/kudu/tools/tool_action_table.cc
@@ -1802,6 +1802,7 @@
       .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();