Repository: kudu
Updated Branches:
  refs/heads/master 1ae050e4d -> 7b048b8db


KUDU-2191: support table-name identifiers with upper case chars

Summary: When the HMS integration is enabled, Kudu now preserves table
name casing, but uses case-insensitive lookups to retrieve tables.

Background: The HMS lowercases all database (table) identifiers during database
(table) creation, only storing the lowercased version. On database and
table lookup the HMS automatically does a case-insensitive compare.
During table creation Kudu checks that table names are valid UTF-8, and
does no transformations on identiers. During table lookups Kudu requires
that the table name match exactly, including case.

As a result of these behavior differences and the design of the
notification log listener, tables with upper-case characters can not be
altered or deleted when the HMS integration is enabled. This commit
fixes this by changing how the Catalog Manager handles identifiers when
the HMS integration is enabled:

* During table creation, the Catalog Manager preserves the case of table
  names.
* On table lookup, the Catalog Manager does a case-insensitive
  comparison to find the table.

This is implemented by storing the preserved case in the table's
sys-catalog metadata entry, and storing a 'normalized' (down-cased)
identifier in the ephemeral by-name table map. The various parts of the
catalog manager which deal with the by-name map are converted to use the
normalized version of the name. When the HMS integration is not
configured, normalized table names are equal to the original table name,
so the behavior changes that this patch introduces are entirely opt-in.

There is one edge case that complicates turning on the HMS integration
in rare circumstances: if there are existing (legacy) tables with names
which map to the same normalized form (e.g. differ only in case), the
catalog manager will fail to startup and instruct the operator to rename
the offending tables before trying again. Additionally, this check only
applies to tables that otherwise follow the Hive table naming rules
(matching regex '[\w_/]+\.[\w_/]+').

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

Branch: refs/heads/master
Commit: 7b048b8dbe2562e68dd2b2b7f1a4db83b1ae10bf
Parents: 1ae050e
Author: Dan Burkert <danburk...@apache.org>
Authored: Fri Jun 22 14:19:38 2018 -0700
Committer: Dan Burkert <danburk...@apache.org>
Committed: Tue Jul 3 23:23:32 2018 +0000

----------------------------------------------------------------------
 src/kudu/hms/hms_catalog-test.cc                |  62 +++++---
 src/kudu/hms/hms_catalog.cc                     |  97 +++++++++----
 src/kudu/hms/hms_catalog.h                      |  26 +++-
 src/kudu/hms/hms_client-test.cc                 |  28 ++++
 .../integration-tests/master-stress-test.cc     |   2 +-
 src/kudu/integration-tests/master_hms-itest.cc  | 126 +++++++++++++++--
 src/kudu/master/catalog_manager.cc              | 141 +++++++++++++------
 src/kudu/master/catalog_manager.h               |  34 +++--
 src/kudu/mini-cluster/external_mini_cluster.cc  |  15 ++
 src/kudu/mini-cluster/external_mini_cluster.h   |   8 +-
 10 files changed, 422 insertions(+), 117 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/7b048b8d/src/kudu/hms/hms_catalog-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/hms/hms_catalog-test.cc b/src/kudu/hms/hms_catalog-test.cc
index 12066f6..472f3a8 100644
--- a/src/kudu/hms/hms_catalog-test.cc
+++ b/src/kudu/hms/hms_catalog-test.cc
@@ -37,6 +37,7 @@
 #include "kudu/rpc/sasl_common.h"
 #include "kudu/security/test/mini_kdc.h"
 #include "kudu/util/net/net_util.h"
+#include "kudu/util/slice.h"
 #include "kudu/util/status.h"
 #include "kudu/util/test_macros.h"
 #include "kudu/util/test_util.h"
@@ -55,35 +56,64 @@ namespace kudu {
 namespace hms {
 
 TEST(HmsCatalogStaticTest, TestParseTableName) {
-  string db;
-  string tbl;
+  Slice db;
+  Slice tbl;
+  string table;
 
-  EXPECT_OK(HmsCatalog::ParseTableName("foo.bar", &db, &tbl));
+  table = "foo.bar";
+  ASSERT_OK(HmsCatalog::ParseTableName(table, &db, &tbl));
   EXPECT_EQ("foo", db);
   EXPECT_EQ("bar", tbl);
 
-  EXPECT_OK(HmsCatalog::ParseTableName("99bottles.my_awesome/table/22", &db, 
&tbl));
+  table = "99bottles.my_awesome/table/22";
+  ASSERT_OK(HmsCatalog::ParseTableName(table, &db, &tbl));
   EXPECT_EQ("99bottles", db);
   EXPECT_EQ("my_awesome/table/22", tbl);
 
-  
EXPECT_OK(HmsCatalog::ParseTableName("_leading_underscore.trailing_underscore_",
 &db, &tbl));
+  table = "_leading_underscore.trailing_underscore_";
+  ASSERT_OK(HmsCatalog::ParseTableName(table, &db, &tbl));
   EXPECT_EQ("_leading_underscore", db);
   EXPECT_EQ("trailing_underscore_", tbl);
 
-  EXPECT_OK(HmsCatalog::ParseTableName("unicode ☃ tables?.maybe/one_day", 
&db, &tbl));
-  EXPECT_EQ("unicode ☃ tables?", db);
-  EXPECT_EQ("maybe/one_day", tbl);
+  EXPECT_TRUE(HmsCatalog::ParseTableName(".", &db, &tbl).IsInvalidArgument());
+  EXPECT_TRUE(HmsCatalog::ParseTableName("no-table", &db, 
&tbl).IsInvalidArgument());
+  EXPECT_TRUE(HmsCatalog::ParseTableName("lots.of.tables", &db, 
&tbl).IsInvalidArgument());
+  EXPECT_TRUE(HmsCatalog::ParseTableName("no-table", &db, 
&tbl).IsInvalidArgument());
+  EXPECT_TRUE(HmsCatalog::ParseTableName("lots.of.tables", &db, 
&tbl).IsInvalidArgument());
+  EXPECT_TRUE(HmsCatalog::ParseTableName(".no_table", &db, 
&tbl).IsInvalidArgument());
+  EXPECT_TRUE(HmsCatalog::ParseTableName(".no_database", &db, 
&tbl).IsInvalidArgument());
+  EXPECT_TRUE(HmsCatalog::ParseTableName("punctuation?.no", &db, 
&tbl).IsInvalidArgument());
+  EXPECT_TRUE(HmsCatalog::ParseTableName("white space.no", &db, 
&tbl).IsInvalidArgument());
+  EXPECT_TRUE(HmsCatalog::ParseTableName("unicode☃tables.no", &db, 
&tbl).IsInvalidArgument());
+  EXPECT_TRUE(HmsCatalog::ParseTableName(string("\0.\0", 3), &db, 
&tbl).IsInvalidArgument());
+}
 
-  EXPECT_OK(HmsCatalog::ParseTableName(".", &db, &tbl));
-  EXPECT_EQ("", db);
-  EXPECT_EQ("", tbl);
+TEST(HmsCatalogStaticTest, TestNormalizeTableName) {
+  string table = "foo.bar";
+  ASSERT_OK(HmsCatalog::NormalizeTableName(&table));
+  ASSERT_EQ("foo.bar", table);
 
-  EXPECT_OK(HmsCatalog::ParseTableName(string("\0.\0", 3), &db, &tbl));
-  EXPECT_EQ(string("\0", 1), db);
-  EXPECT_EQ(string("\0", 1), tbl);
+  table = "fOo.BaR";
+  ASSERT_OK(HmsCatalog::NormalizeTableName(&table));
+  EXPECT_EQ("foo.bar", table);
 
-  EXPECT_TRUE(HmsCatalog::ParseTableName("no-table", &db, 
&tbl).IsInvalidArgument());
-  EXPECT_TRUE(HmsCatalog::ParseTableName("lots.of.tables", &db, 
&tbl).IsInvalidArgument());
+  table = "A.B";
+  ASSERT_OK(HmsCatalog::NormalizeTableName(&table));
+  EXPECT_EQ("a.b", table);
+
+  table = "__/A__.buzz";
+  ASSERT_OK(HmsCatalog::NormalizeTableName(&table));
+  EXPECT_EQ("__/a__.buzz", table);
+
+  table = "THE/QUICK/BROWN/FOX/JUMPS/OVER/THE/LAZY/DOG."
+          "the_quick_brown_fox_jumps_over_the_lazy_dog";
+  ASSERT_OK(HmsCatalog::NormalizeTableName(&table));
+  EXPECT_EQ("the/quick/brown/fox/jumps/over/the/lazy/dog."
+            "the_quick_brown_fox_jumps_over_the_lazy_dog", table);
+
+  table = "default.MyTable";
+  ASSERT_OK(HmsCatalog::NormalizeTableName(&table));
+  EXPECT_EQ("default.mytable", table);
 }
 
 TEST(HmsCatalogStaticTest, TestParseUris) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/7b048b8d/src/kudu/hms/hms_catalog.cc
----------------------------------------------------------------------
diff --git a/src/kudu/hms/hms_catalog.cc b/src/kudu/hms/hms_catalog.cc
index 0bc44bb..fe904cd 100644
--- a/src/kudu/hms/hms_catalog.cc
+++ b/src/kudu/hms/hms_catalog.cc
@@ -26,6 +26,7 @@
 #include <utility>
 #include <vector>
 
+#include <boost/optional/optional.hpp>
 #include <gflags/gflags.h>
 #include <glog/logging.h>
 
@@ -33,6 +34,8 @@
 #include "kudu/common/schema.h"
 #include "kudu/common/types.h"
 #include "kudu/gutil/macros.h"
+#include "kudu/gutil/strings/ascii_ctype.h"
+#include "kudu/gutil/strings/charset.h"
 #include "kudu/gutil/strings/split.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/hms/hive_metastore_types.h"
@@ -43,6 +46,7 @@
 #include "kudu/util/slice.h"
 #include "kudu/util/threadpool.h"
 
+using boost::optional;
 using std::move;
 using std::string;
 using std::vector;
@@ -97,8 +101,8 @@ namespace kudu {
 namespace hms {
 
 const char* const HmsCatalog::kInvalidTableError = "when the Hive Metastore 
integration "
-    "is enabled, Kudu table names must be a period "
-    "('.') separated database and table name pair";
+    "is enabled, Kudu table names must be a period ('.') separated database 
and table name "
+    "identifier pair, each containing only ASCII alphanumeric characters, '_', 
and '/'";
 
 HmsCatalog::HmsCatalog(string master_addresses)
     : master_addresses_(std::move(master_addresses)),
@@ -147,15 +151,15 @@ Status HmsCatalog::CreateTable(const string& id,
 }
 
 Status HmsCatalog::DropTable(const string& id, const string& name) {
-  string hms_database;
-  string hms_table;
+  Slice hms_database;
+  Slice hms_table;
   RETURN_NOT_OK(ParseTableName(name, &hms_database, &hms_table));
 
   hive::EnvironmentContext env_ctx = EnvironmentContext();
   env_ctx.properties.insert(make_pair(HmsClient::kKuduTableIdKey, id));
 
   return Execute([&] (HmsClient* client) {
-    return client->DropTable(hms_database, hms_table, env_ctx);
+    return client->DropTable(hms_database.ToString(), hms_table.ToString(), 
env_ctx);
   });
 }
 
@@ -179,11 +183,12 @@ Status HmsCatalog::UpgradeLegacyImpalaTable(const string& 
id,
 
 Status HmsCatalog::DowngradeToLegacyImpalaTable(const std::string& name) {
   return Execute([&] (HmsClient* client) {
-    string hms_database;
-    string hms_table;
+    Slice hms_database;
+    Slice hms_table;
     RETURN_NOT_OK(ParseTableName(name, &hms_database, &hms_table));
+
     hive::Table table;
-    RETURN_NOT_OK(client->GetTable(hms_database, hms_table, &table));
+    RETURN_NOT_OK(client->GetTable(hms_database.ToString(), 
hms_table.ToString(), &table));
     if (table.parameters[HmsClient::kStorageHandlerKey] !=
         HmsClient::kKuduStorageHandler) {
       return Status::IllegalState("non-Kudu table cannot be downgraded");
@@ -200,7 +205,7 @@ Status HmsCatalog::DowngradeToLegacyImpalaTable(const 
std::string& name) {
     // Remove the Kudu-specific field 'kudu.table_id'.
     EraseKeyReturnValuePtr(&table.parameters, HmsClient::kKuduTableIdKey);
 
-    return client->AlterTable(hms_database, hms_table, table, 
EnvironmentContext());
+    return client->AlterTable(table.dbName, table.tableName, table, 
EnvironmentContext());
   });
 }
 
@@ -243,12 +248,12 @@ Status HmsCatalog::AlterTable(const string& id,
       // - The original table does not exist in the HMS
       // - The original table doesn't match the Kudu table being altered
 
-      string hms_database;
-      string hms_table;
+      Slice hms_database;
+      Slice hms_table;
       RETURN_NOT_OK(ParseTableName(name, &hms_database, &hms_table));
 
       hive::Table table;
-      RETURN_NOT_OK(client->GetTable(hms_database, hms_table, &table));
+      RETURN_NOT_OK(client->GetTable(hms_database.ToString(), 
hms_table.ToString(), &table));
 
       // Check that the HMS entry belongs to the table being altered.
       if (table.parameters[HmsClient::kStorageHandlerKey] != 
HmsClient::kKuduStorageHandler ||
@@ -260,7 +265,8 @@ Status HmsCatalog::AlterTable(const string& id,
 
       // Overwrite fields in the table that have changed, including the new 
name.
       RETURN_NOT_OK(PopulateTable(id, new_name, schema, master_addresses_, 
&table));
-      return client->AlterTable(hms_database, hms_table, table, 
EnvironmentContext());
+      return client->AlterTable(hms_database.ToString(), hms_table.ToString(),
+                                table, EnvironmentContext());
   });
 }
 
@@ -447,6 +453,12 @@ hive::FieldSchema column_to_field(const ColumnSchema& 
column) {
   return field;
 }
 
+// Convert an ASCII encoded string to lowercase in place.
+void ToLowerCase(Slice s) {
+  for (int i = 0; i < s.size(); i++) {
+    s.mutable_data()[i] = ascii_tolower(s[i]);
+  }
+}
 } // anonymous namespace
 
 Status HmsCatalog::PopulateTable(const string& id,
@@ -454,7 +466,11 @@ Status HmsCatalog::PopulateTable(const string& id,
                                  const Schema& schema,
                                  const string& master_addresses,
                                  hive::Table* table) {
-  RETURN_NOT_OK(ParseTableName(name, &table->dbName, &table->tableName));
+  Slice hms_database_name;
+  Slice hms_table_name;
+  RETURN_NOT_OK(ParseTableName(name, &hms_database_name, &hms_table_name));
+  table->dbName = hms_database_name.ToString();
+  table->tableName = hms_table_name.ToString();
 
   // Add the Kudu-specific parameters. This intentionally avoids overwriting
   // other parameters.
@@ -483,24 +499,45 @@ Status HmsCatalog::PopulateTable(const string& id,
   return Status::OK();
 }
 
-Status HmsCatalog::ParseTableName(const string& table,
-                                  string* hms_database,
-                                  string* hms_table) {
-  // We do minimal parsing or validating of the identifiers, since Hive has
-  // different validation rules based on configuration (and probably version).
-  // The only rule we enforce is that there be exactly one period to separate
-  // the database and table names, we leave checking of everything else to the
-  // HMS.
-  //
-  // See org.apache.hadoop.hive.metastore.MetaStoreUtils.validateName.
-
-  vector<string> identifiers = strings::Split(table, ".");
-  if (identifiers.size() != 2) {
-    return Status::InvalidArgument(kInvalidTableError, table);
+Status HmsCatalog::NormalizeTableName(string* table_name) {
+  CHECK_NOTNULL(table_name);
+  Slice hms_database;
+  Slice hms_table;
+  RETURN_NOT_OK(ParseTableName(*table_name, &hms_database, &hms_table));
+
+  ToLowerCase(hms_database);
+  ToLowerCase(hms_table);
+
+  return Status::OK();
+}
+
+Status HmsCatalog::ParseTableName(const string& table_name,
+                                  Slice* hms_database,
+                                  Slice* hms_table) {
+  const char kSeparator = '.';
+  strings::CharSet charset("abcdefghijklmnopqrstuvwxyz"
+                           "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+                           "0123456789"
+                           "_/");
+
+  optional<int> separator_idx;
+  for (int idx = 0; idx < table_name.size(); idx++) {
+    char c = table_name[idx];
+    if (!charset.Test(c)) {
+      if (c == kSeparator && !separator_idx) {
+        separator_idx = idx;
+      } else {
+        return Status::InvalidArgument(kInvalidTableError, table_name);
+      }
+    }
+  }
+  if (!separator_idx || *separator_idx == 0 || *separator_idx == 
table_name.size() - 1) {
+    return Status::InvalidArgument(kInvalidTableError, table_name);
   }
 
-  *hms_database = std::move(identifiers[0]);
-  *hms_table = std::move(identifiers[1]);
+  *hms_database = Slice(table_name.data(), *separator_idx);
+  *hms_table = Slice(table_name.data() + *separator_idx + 1,
+                     table_name.size() - *separator_idx - 1);
   return Status::OK();
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/7b048b8d/src/kudu/hms/hms_catalog.h
----------------------------------------------------------------------
diff --git a/src/kudu/hms/hms_catalog.h b/src/kudu/hms/hms_catalog.h
index b9f61f1..d40fb3c 100644
--- a/src/kudu/hms/hms_catalog.h
+++ b/src/kudu/hms/hms_catalog.h
@@ -30,6 +30,8 @@
 #include "kudu/util/net/net_util.h"
 #include "kudu/util/status.h"
 
+class StringPiece;
+
 namespace hive {
 class NotificationEvent;
 class Table;
@@ -129,9 +131,26 @@ class HmsCatalog {
                               const std::string& master_addresses,
                               hive::Table* table) WARN_UNUSED_RESULT;
 
+  // Validates and canonicalizes the provided table name according to HMS 
rules.
+  // If the table name is not valid it will not be modified. If the table name
+  // is valid, it will be canonicalized.
+  //
+  // Valid Kudu/HMS table names consist of a period ('.') separated database 
and
+  // table name pair. The database and table names must contain only the ASCII
+  // alphanumeric, '_', and '/' characters.
+  //
+  // Normalized Kudu/HMS table names are downcased so that they contain no
+  // upper-case (A-Z) ASCII characters.
+  //
+  // Hive handles validating and canonicalizing table names in
+  // org.apache.hadoop.hive.metastore.MetaStoreUtils.validateName and
+  // org.apache.hadoop.hive.common.util.normalizeIdentifier.
+  static Status NormalizeTableName(std::string* table_name);
+
  private:
 
   FRIEND_TEST(HmsCatalogStaticTest, TestParseTableName);
+  FRIEND_TEST(HmsCatalogStaticTest, TestParseTableNameSlices);
   FRIEND_TEST(HmsCatalogStaticTest, TestParseUris);
 
   // Synchronously executes a task with exclusive access to the HMS client.
@@ -148,9 +167,10 @@ class HmsCatalog {
 
   // Parses a Kudu table name into a Hive database and table name.
   // Returns an error if the Kudu table name is not correctly formatted.
-  static Status ParseTableName(const std::string& table,
-                               std::string* hms_database,
-                               std::string* hms_table) WARN_UNUSED_RESULT;
+  // The returned HMS database and table slices must not outlive 'table_name'.
+  static Status ParseTableName(const std::string& table_name,
+                               Slice* hms_database,
+                               Slice* hms_table) WARN_UNUSED_RESULT;
 
   // Parses a Hive Metastore URI string into a sequence of HostPorts.
   static Status ParseUris(const std::string& metastore_uris, 
std::vector<HostPort>* hostports);

http://git-wip-us.apache.org/repos/asf/kudu/blob/7b048b8d/src/kudu/hms/hms_client-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/hms/hms_client-test.cc b/src/kudu/hms/hms_client-test.cc
index 8e1dcd7..2cb6a4c 100644
--- a/src/kudu/hms/hms_client-test.cc
+++ b/src/kudu/hms/hms_client-test.cc
@@ -403,5 +403,33 @@ TEST_F(HmsClientTest, TestDeserializeJsonTable) {
   ASSERT_EQ("database_name", table.dbName);
 }
 
+TEST_F(HmsClientTest, TestCaseSensitivity) {
+  MiniKdc kdc;
+  MiniHms hms;
+  ASSERT_OK(hms.Start());
+
+  HmsClient client(hms.address(), HmsClientOptions());
+  ASSERT_OK(client.Start());
+
+  // Create a database.
+  hive::Database db;
+  db.name = "my_db";
+  ASSERT_OK(client.CreateDatabase(db));
+
+  // Create a table.
+  ASSERT_OK(CreateTable(&client, "my_db", "Foo", "abc123"));
+
+  hive::Table table;
+  ASSERT_OK(client.GetTable("my_db", "Foo", &table));
+  ASSERT_EQ(table.tableName, "foo");
+
+  ASSERT_OK(client.GetTable("my_db", "foo", &table));
+  ASSERT_EQ(table.tableName, "foo");
+
+  ASSERT_OK(client.GetTable("MY_DB", "FOO", &table));
+  ASSERT_EQ(table.dbName, "my_db");
+  ASSERT_EQ(table.tableName, "foo");
+}
+
 } // namespace hms
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/7b048b8d/src/kudu/integration-tests/master-stress-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/master-stress-test.cc 
b/src/kudu/integration-tests/master-stress-test.cc
index 53040d6..38aa71f 100644
--- a/src/kudu/integration-tests/master-stress-test.cc
+++ b/src/kudu/integration-tests/master-stress-test.cc
@@ -396,7 +396,7 @@ class MasterStressTest : public KuduTest,
 
  private:
   string GenerateTableName() {
-    return Substitute("default.table_$0", oid_generator_.Next());
+    return Substitute("default.Table_$0", oid_generator_.Next());
   }
 
   bool BlockingGetTableName(string* chosen_table) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/7b048b8d/src/kudu/integration-tests/master_hms-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/master_hms-itest.cc 
b/src/kudu/integration-tests/master_hms-itest.cc
index 8a7c4a1..4b639f5 100644
--- a/src/kudu/integration-tests/master_hms-itest.cc
+++ b/src/kudu/integration-tests/master_hms-itest.cc
@@ -32,10 +32,12 @@
 #include "kudu/common/common.pb.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/hms/hive_metastore_types.h"
+#include "kudu/hms/hms_catalog.h"
 #include "kudu/hms/hms_client.h"
 #include "kudu/hms/mini_hms.h"
 #include "kudu/integration-tests/external_mini_cluster-itest-base.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"
+#include "kudu/mini-cluster/mini_cluster.h"
 #include "kudu/util/decimal_util.h"
 #include "kudu/util/net/net_util.h"
 #include "kudu/util/status.h"
@@ -63,11 +65,15 @@ using strings::Substitute;
 class MasterHmsTest : public ExternalMiniClusterITestBase {
  public:
 
+  virtual HmsMode GetHmsMode() {
+    return HmsMode::ENABLE_METASTORE_INTEGRATION;
+  }
+
   void SetUp() override {
     ExternalMiniClusterITestBase::SetUp();
 
     ExternalMiniClusterOptions opts;
-    opts.hms_mode = HmsMode::ENABLE_METASTORE_INTEGRATION;
+    opts.hms_mode = GetHmsMode();
     opts.num_masters = 1;
     opts.num_tablet_servers = 1;
     // Tune down the notification log poll period in order to speed up catalog 
convergence.
@@ -234,7 +240,7 @@ TEST_F(MasterHmsTest, TestCreateTable) {
   // Attempt to create a Kudu table to an invalid table name.
   s = CreateKuduTable(hms_database_name, "☃");
   ASSERT_TRUE(s.IsInvalidArgument()) << s.ToString();
-  ASSERT_STR_CONTAINS(s.ToString(), "☃ is not a valid object name");
+  ASSERT_STR_CONTAINS(s.ToString(), "create_db.☃");
 
   // Drop the HMS entry and create the table through Kudu.
   ASSERT_OK(hms_client_->DropTable(hms_database_name, hms_table_name));
@@ -284,8 +290,7 @@ TEST_F(MasterHmsTest, TestRenameTable) {
   table_alterer.reset(client_->NewTableAlterer("db.a"));
   s = table_alterer->RenameTo("foo")->Alter();
   ASSERT_TRUE(s.IsInvalidArgument()) << s.ToString();
-  ASSERT_STR_CONTAINS(s.ToString(), "Kudu table names must be a period ('.') 
separated "
-                                    "database and table name pair");
+  ASSERT_STR_CONTAINS(s.ToString(), hms::HmsCatalog::kInvalidTableError);
 
   // Attempt to rename the Kudu table to a non-existent database.
   table_alterer.reset(client_->NewTableAlterer("db.a"));
@@ -296,8 +301,8 @@ TEST_F(MasterHmsTest, TestRenameTable) {
   // Attempt to rename the Kudu table to an invalid table name.
   table_alterer.reset(client_->NewTableAlterer("db.a"));
   s = table_alterer->RenameTo("db.☃")->Alter();
-  ASSERT_TRUE(s.IsIllegalState()) << s.ToString();
-  ASSERT_STR_CONTAINS(s.ToString(), "☃ is not a valid object name");
+  ASSERT_TRUE(s.IsInvalidArgument()) << s.ToString();
+  ASSERT_STR_CONTAINS(s.ToString(), "db.☃");
 
   // Shutdown the HMS and try to rename the table.
   ASSERT_OK(StopHms());
@@ -467,13 +472,118 @@ TEST_F(MasterHmsTest, TestNotificationLogListener) {
   ASSERT_OK(hms_client_->DropTable("default", "a"));
   Status s = client_->DeleteTable("default.a");
   ASSERT_TRUE(s.IsNotFound()) << s.ToString();
-  CheckTableDoesNotExist("default", "a");
+  NO_FATALS(CheckTableDoesNotExist("default", "a"));
 
   // Scenario 2: drop from Kudu first.
   ASSERT_OK(CreateKuduTable("default", "a"));
   ASSERT_OK(client_->DeleteTable("default.a"));
   s = hms_client_->DropTable("default", "a");
   ASSERT_TRUE(s.IsNotFound()) << s.ToString();
-  CheckTableDoesNotExist("default", "a");
+  NO_FATALS(CheckTableDoesNotExist("default", "a"));
+}
+
+TEST_F(MasterHmsTest, TestUppercaseIdentifiers) {
+  ASSERT_OK(CreateKuduTable("default", "MyTable"));
+  NO_FATALS(CheckTable("default", "MyTable"));
+  NO_FATALS(CheckTable("default", "mytable"));
+  NO_FATALS(CheckTable("default", "MYTABLE"));
+
+  // Kudu table schema lookups should be case-insensitive.
+  for (const auto& name : { "default.MyTable",
+                            "default.mytable",
+                            "DEFAULT.MYTABLE",
+                            "default.mYtABLE" }) {
+    shared_ptr<KuduTable> table;
+    ASSERT_OK(client_->OpenTable(name, &table));
+    // The client uses the requested name as the table object's name.
+    ASSERT_EQ(name, table->name());
+  }
+
+  // Listing tables shows the preserved case.
+  vector<string> tables;
+  ASSERT_OK(client_->ListTables(&tables));
+  ASSERT_EQ(tables, vector<string>({ "default.MyTable" }));
+
+  // Rename the table to the same normalized name, but with a different case.
+  unique_ptr<KuduTableAlterer> table_alterer;
+  table_alterer.reset(client_->NewTableAlterer("default.mytable"));
+  ASSERT_OK(table_alterer->RenameTo("DEFAULT.MYTABLE")->Alter());
+  NO_FATALS(CheckTable("default", "MyTable"));
+  NO_FATALS(CheckTable("default", "mytable"));
+  NO_FATALS(CheckTable("default", "MYTABLE"));
+
+  // The master should retain the new case.
+  tables.clear();
+  ASSERT_OK(client_->ListTables(&tables));
+  ASSERT_EQ(tables, vector<string>({ "DEFAULT.MYTABLE" }));
+
+  // Rename the table to something different.
+  table_alterer.reset(client_->NewTableAlterer("DEFAULT.MYTABLE"));
+  ASSERT_OK(table_alterer->RenameTo("default.T_1/1")->Alter());
+  NO_FATALS(CheckTable("default", "T_1/1"));
+  NO_FATALS(CheckTable("default", "t_1/1"));
+  NO_FATALS(CheckTable("DEFAULT", "T_1/1"));
+
+  // Rename the table through the HMS.
+  ASSERT_OK(RenameHmsTable("default", "T_1/1", "AbC"));
+  ASSERT_EVENTUALLY([&] {
+    NO_FATALS(CheckTable("default", "AbC"));
+  });
+
+  // Listing tables shows the preserved case.
+  tables.clear();
+  ASSERT_OK(client_->ListTables(&tables));
+  ASSERT_EQ(tables, vector<string>({ "default.AbC" }));
+
+  // Drop the table.
+  ASSERT_OK(client_->DeleteTable("DEFAULT.abc"));
+  NO_FATALS(CheckTableDoesNotExist("default", "AbC"));
+  NO_FATALS(CheckTableDoesNotExist("default", "abc"));
+}
+
+class MasterHmsUpgradeTest : public MasterHmsTest {
+ public:
+  HmsMode GetHmsMode() override {
+    return HmsMode::ENABLE_HIVE_METASTORE;
+  }
+};
+
+TEST_F(MasterHmsUpgradeTest, TestConflictingNormalizedNames) {
+  ASSERT_OK(CreateKuduTable("default", "MyTable"));
+  ASSERT_OK(CreateKuduTable("default", "mytable"));
+
+  // Shutdown the masters and turn on the HMS integration. The masters should
+  // fail to startup because of conflicting normalized table names.
+  cluster_->ShutdownNodes(cluster::ClusterNodes::MASTERS_ONLY);
+  cluster_->EnableMetastoreIntegration();
+  // Typically, restarting the cluster will fail because the master will
+  // immediately try to elect itself leader, and CHECK fail upon seeing the
+  // conflicting table names. However, in TSAN or otherwise slow situations the
+  // master may be able to register the tablet server before attempting to 
elect
+  // itself leader, in which case ExternalMiniCluster::Restart() can succeed. 
In
+  // this situation a fallback to a leader-only API will deterministically 
fail.
+  Status s = cluster_->Restart();
+  if (s.ok()) {
+    vector<string> tables;
+    s = client_->ListTables(&tables);
+  }
+  ASSERT_TRUE(s.IsNetworkError()) << s.ToString();
+
+  // Disable the metastore integration and rename one of the conflicting 
tables.
+  cluster_->ShutdownNodes(cluster::ClusterNodes::MASTERS_ONLY);
+  cluster_->DisableMetastoreIntegration();
+  ASSERT_OK(cluster_->Restart());
+  unique_ptr<KuduTableAlterer> 
table_alterer(client_->NewTableAlterer("default.mytable"));
+  ASSERT_OK(table_alterer->RenameTo("default.mytable-renamed")->Alter());
+
+  // Try again to enable the integration.
+  cluster_->ShutdownNodes(cluster::ClusterNodes::MASTERS_ONLY);
+  cluster_->EnableMetastoreIntegration();
+  ASSERT_OK(cluster_->Restart());
+
+  vector<string> tables;
+  client_->ListTables(&tables);
+  std::sort(tables.begin(), tables.end());
+  ASSERT_EQ(tables, vector<string>({ "default.MyTable", 
"default.mytable-renamed" }));
 }
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/7b048b8d/src/kudu/master/catalog_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.cc 
b/src/kudu/master/catalog_manager.cc
index 7094a70..8f58924 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -315,7 +315,18 @@ class TableLoader : public TableVisitor {
     bool is_deleted = l.mutable_data()->is_deleted();
     catalog_manager_->table_ids_map_[table->id()] = table;
     if (!is_deleted) {
-      catalog_manager_->table_names_map_[l.data().name()] = table;
+      auto* existing = 
InsertOrReturnExisting(&catalog_manager_->normalized_table_names_map_,
+                                              
CatalogManager::NormalizeTableName(l.data().name()),
+                                              table);
+      if (existing) {
+        // Return an HMS-specific error message, since this error currently 
only
+        // occurs when the HMS is enabled.
+        return Status::IllegalState(
+            "when the Hive Metastore integration is enabled, Kudu table names 
must not differ "
+            "only by case; restart the master(s) with the Hive Metastore 
integration disabled and "
+            "rename one of the conflicting tables",
+            Substitute("$0 or $1 [id=$2]", (*existing)->ToString(), 
l.data().name(), table_id));
+      }
     }
     l.Commit();
 
@@ -1126,7 +1137,7 @@ Status CatalogManager::VisitTablesAndTabletsUnlocked() {
   AbortAndWaitForAllTasks(tables);
 
   // Clear the existing state.
-  table_names_map_.clear();
+  normalized_table_names_map_.clear();
   table_ids_map_.clear();
   tablet_map_.clear();
 
@@ -1353,6 +1364,7 @@ Status CatalogManager::CreateTable(const 
CreateTableRequestPB* orig_req,
   Schema client_schema;
   RETURN_NOT_OK(SchemaFromPB(req.schema(), &client_schema));
   const string& table_name = req.name();
+  string normalized_table_name = NormalizeTableName(table_name);
 
   RETURN_NOT_OK(SetupError(ValidateClientSchema(table_name, client_schema),
                            resp, MasterErrorPB::INVALID_SCHEMA));
@@ -1494,7 +1506,7 @@ Status CatalogManager::CreateTable(const 
CreateTableRequestPB* orig_req,
     TRACE("Acquired catalog manager lock");
 
     // b. Verify that the table does not exist.
-    table = FindPtrOrNull(table_names_map_, table_name);
+    table = FindPtrOrNull(normalized_table_names_map_, normalized_table_name);
     if (table != nullptr) {
       return SetupError(Status::AlreadyPresent(Substitute(
               "table $0 already exists with id $1", table_name, table->id())),
@@ -1502,7 +1514,7 @@ Status CatalogManager::CreateTable(const 
CreateTableRequestPB* orig_req,
     }
 
     // c. Reserve the table name if possible.
-    if (!InsertIfNotPresent(&reserved_table_names_, table_name)) {
+    if (!InsertIfNotPresent(&reserved_normalized_table_names_, 
normalized_table_name)) {
       // ServiceUnavailable will cause the client to retry the create table
       // request. We don't want to outright fail the request with
       // 'AlreadyPresent', because a table name reservation can be rolled back
@@ -1517,7 +1529,7 @@ Status CatalogManager::CreateTable(const 
CreateTableRequestPB* orig_req,
   // Ensure that we drop the name reservation upon return.
   SCOPED_CLEANUP({
     std::lock_guard<LockType> l(lock_);
-    CHECK_EQ(1, reserved_table_names_.erase(table_name));
+    CHECK_EQ(1, reserved_normalized_table_names_.erase(normalized_table_name));
   });
 
   // d. Create the in-memory representation of the new table and its tablets.
@@ -1610,7 +1622,7 @@ Status CatalogManager::CreateTable(const 
CreateTableRequestPB* orig_req,
     std::lock_guard<LockType> l(lock_);
 
     table_ids_map_[table->id()] = table;
-    table_names_map_[table_name] = table;
+    normalized_table_names_map_[normalized_table_name] = table;
     for (const auto& tablet : tablets) {
       InsertOrDie(&tablet_map_, tablet->id(), tablet);
     }
@@ -1695,11 +1707,13 @@ Status CatalogManager::FindAndLockTable(const ReqClass& 
request,
       // If the request contains both a table ID and table name, ensure that
       // both match the same table.
       if (table_identifier.has_table_name() &&
-          table.get() != FindPtrOrNull(table_names_map_, 
table_identifier.table_name()).get()) {
+          table.get() != FindPtrOrNull(normalized_table_names_map_,
+                                       
NormalizeTableName(table_identifier.table_name())).get()) {
         return tnf_error();
       }
     } else if (table_identifier.has_table_name()) {
-      table = FindPtrOrNull(table_names_map_, table_identifier.table_name());
+      table = FindPtrOrNull(normalized_table_names_map_,
+                            NormalizeTableName(table_identifier.table_name()));
     } else {
       return SetupError(Status::InvalidArgument("missing table ID or table 
name"),
                         response, MasterErrorPB::UNKNOWN_ERROR);
@@ -1714,7 +1728,8 @@ Status CatalogManager::FindAndLockTable(const ReqClass& 
request,
   // Acquire the table lock.
   TableMetadataLock lock(table.get(), lock_mode);
 
-  if (table_identifier.has_table_name() && table_identifier.table_name() != 
lock.data().name()) {
+  if (table_identifier.has_table_name() &&
+      NormalizeTableName(table_identifier.table_name()) != 
NormalizeTableName(lock.data().name())) {
     // We've encountered the table while it's in the process of being renamed;
     // pretend it doesn't yet exist.
     return tnf_error();
@@ -1843,7 +1858,7 @@ Status CatalogManager::DeleteTable(const 
DeleteTableRequestPB& req,
     {
       TRACE("Removing table from by-name map");
       std::lock_guard<LockType> l_map(lock_);
-      if (table_names_map_.erase(l.data().name()) != 1) {
+      if 
(normalized_table_names_map_.erase(NormalizeTableName(l.data().name())) != 1) {
         LOG(FATAL) << "Could not remove table " << table->ToString()
                    << " from map in response to DeleteTable request: "
                    << SecureShortDebugString(req);
@@ -2143,17 +2158,16 @@ Status CatalogManager::AlterTableRpc(const 
AlterTableRequestPB& req,
   // that event to the catalog. By 'serializing' the rename through the
   // HMS, race conditions are avoided.
   if (hms_catalog_ && req.has_new_table_name() && 
req.alter_external_catalogs()) {
-    // Look up the table, lock it, and mark it as removed.
+    // Look up the table and lock it.
     scoped_refptr<TableInfo> table;
     TableMetadataLock l;
     RETURN_NOT_OK(FindAndLockTable(req, resp, LockMode::READ, &table, &l));
     RETURN_NOT_OK(CheckIfTableDeletedOrNotRunning(&l, resp));
 
-    // The HMS allows renaming a table to the same name (ALTER TABLE t RENAME 
TO t;),
-    // however Kudu does not, so we must validate this case ourselves. This
-    // invariant is also checked in CatalogManager::AlterTable, but when the 
HMS
-    // integration is enabled that check does not bubble up to the client (it's
-    // called by the notification log listener).
+    // The HMS allows renaming a table to the same name (ALTER TABLE t RENAME 
TO t),
+    // however Kudu does not, so we must enforce this constraint ourselves 
before
+    // altering the table in the HMS. The comparison is on the non-normalized
+    // table names, since we want to allow changing the case of a table name.
     if (l.data().name() == req.new_table_name()) {
       return SetupError(
           Status::AlreadyPresent(Substitute("table $0 already exists with id 
$1",
@@ -2248,6 +2262,7 @@ Status CatalogManager::AlterTable(const 
AlterTableRequestPB& req,
   }
 
   string table_name = l.data().name();
+  string normalized_table_name = NormalizeTableName(table_name);
   *resp->mutable_table_id() = table->id();
 
   // 3. Calculate and validate new schema for the on-disk state, not persisted 
yet.
@@ -2272,33 +2287,47 @@ Status CatalogManager::AlterTable(const 
AlterTableRequestPB& req,
         resp, MasterErrorPB::INVALID_SCHEMA));
 
   // 4. Validate and try to acquire the new table name.
+  bool do_cleanup = false;
+  string normalized_new_table_name = NormalizeTableName(req.new_table_name());
   if (req.has_new_table_name()) {
     RETURN_NOT_OK(SetupError(
           ValidateIdentifier(req.new_table_name()).CloneAndPrepend("invalid 
table name"),
           resp, MasterErrorPB::INVALID_SCHEMA));
 
-    std::lock_guard<LockType> catalog_lock(lock_);
-    TRACE("Acquired catalog manager lock");
-
-    // Verify that the table does not exist.
-    scoped_refptr<TableInfo> other_table = FindPtrOrNull(table_names_map_, 
req.new_table_name());
-    if (other_table != nullptr) {
-      return SetupError(
-          Status::AlreadyPresent(Substitute("table $0 already exists with id 
$1",
-              req.new_table_name(), table->id())),
-          resp, MasterErrorPB::TABLE_ALREADY_PRESENT);
-    }
+    // Validate the new table name.
+    //
+    // Special case: the new table name and existing table names are different,
+    // but map to the same normalized name. In this case we don't need to
+    // reserve the new table name, since it's already reserved by way of
+    // existing in the by-name map.
+    if (!(table_name != req.new_table_name() &&
+          normalized_table_name == normalized_new_table_name)) {
+      std::lock_guard<LockType> catalog_lock(lock_);
+      TRACE("Acquired catalog manager lock");
+
+      // Verify that the table does not exist.
+      scoped_refptr<TableInfo> other_table = 
FindPtrOrNull(normalized_table_names_map_,
+                                                           
normalized_new_table_name);
+
+      if (other_table != nullptr) {
+        return SetupError(
+            Status::AlreadyPresent(Substitute("table $0 already exists with id 
$1",
+                req.new_table_name(), table->id())),
+            resp, MasterErrorPB::TABLE_ALREADY_PRESENT);
+      }
 
-    // Reserve the new table name if possible.
-    if (!InsertIfNotPresent(&reserved_table_names_, req.new_table_name())) {
-      // ServiceUnavailable will cause the client to retry the create table
-      // request. We don't want to outright fail the request with
-      // 'AlreadyPresent', because a table name reservation can be rolled back
-      // in the case of an error. Instead, we force the client to retry at a
-      // later time.
-      return SetupError(Status::ServiceUnavailable(Substitute(
-              "table name $0 is already reserved", req.new_table_name())),
-          resp, MasterErrorPB::TABLE_ALREADY_PRESENT);
+      // Reserve the new table name if possible.
+      if (!InsertIfNotPresent(&reserved_normalized_table_names_, 
normalized_new_table_name)) {
+        // ServiceUnavailable will cause the client to retry the create table
+        // request. We don't want to outright fail the request with
+        // 'AlreadyPresent', because a table name reservation can be rolled 
back
+        // in the case of an error. Instead, we force the client to retry at a
+        // later time.
+        return SetupError(Status::ServiceUnavailable(Substitute(
+                "table name $0 is already reserved", req.new_table_name())),
+            resp, MasterErrorPB::TABLE_ALREADY_PRESENT);
+      }
+      do_cleanup = true;
     }
 
     l.mutable_data()->pb.set_name(req.new_table_name());
@@ -2306,9 +2335,9 @@ Status CatalogManager::AlterTable(const 
AlterTableRequestPB& req,
 
   // Ensure that we drop our reservation upon return.
   auto cleanup = MakeScopedCleanup([&] () {
-    if (req.has_new_table_name()) {
+    if (do_cleanup) {
       std::lock_guard<LockType> l(lock_);
-      CHECK_EQ(1, reserved_table_names_.erase(req.new_table_name()));
+      CHECK_EQ(1, 
reserved_normalized_table_names_.erase(normalized_new_table_name));
     }
   });
 
@@ -2405,12 +2434,12 @@ Status CatalogManager::AlterTable(const 
AlterTableRequestPB& req,
     // and tablets indices.
     std::lock_guard<LockType> lock(lock_);
     if (req.has_new_table_name()) {
-      if (table_names_map_.erase(table_name) != 1) {
+      if (normalized_table_names_map_.erase(normalized_table_name) != 1) {
         LOG(FATAL) << "Could not remove table " << table->ToString()
                    << " from map in response to AlterTable request: "
                    << SecureShortDebugString(req);
       }
-      InsertOrDie(&table_names_map_, req.new_table_name(), table);
+      InsertOrDie(&normalized_table_names_map_, normalized_new_table_name, 
table);
     }
 
     // Insert new tablets into the global tablet map. After this, the tablets
@@ -2532,7 +2561,7 @@ Status CatalogManager::ListTables(const 
ListTablesRequestPB* req,
 
   shared_lock<LockType> l(lock_);
 
-  for (const TableInfoMap::value_type& entry : table_names_map_) {
+  for (const TableInfoMap::value_type& entry : normalized_table_names_map_) {
     TableMetadataLock ltm(entry.second.get(), LockMode::READ);
     if (!ltm.data().is_running()) continue; // implies !is_deleted() too
 
@@ -2543,7 +2572,7 @@ Status CatalogManager::ListTables(const 
ListTablesRequestPB* req,
       }
     }
 
-    ListTablesResponsePB::TableInfo *table = resp->add_tables();
+    ListTablesResponsePB::TableInfo* table = resp->add_tables();
     table->set_id(entry.second->id());
     table->set_name(ltm.data().name());
   }
@@ -2576,7 +2605,7 @@ Status CatalogManager::TableNameExists(const string& 
table_name, bool* exists) {
   RETURN_NOT_OK(CheckOnline());
 
   shared_lock<LockType> l(lock_);
-  *exists = ContainsKey(table_names_map_, table_name);
+  *exists = ContainsKey(normalized_table_names_map_, 
NormalizeTableName(table_name));
   return Status::OK();
 }
 
@@ -4674,7 +4703,7 @@ void CatalogManager::DumpState(std::ostream* out) const {
   {
     shared_lock<LockType> l(lock_);
     ids_copy = table_ids_map_;
-    names_copy = table_names_map_;
+    names_copy = normalized_table_names_map_;
     tablets_copy = tablet_map_;
     // TODO(aserbin): add information about root CA certs, if any
   }
@@ -4759,6 +4788,28 @@ Status 
CatalogManager::WaitForNotificationLogListenerCatchUp(RespClass* resp,
   return Status::OK();
 }
 
+string CatalogManager::NormalizeTableName(const string& table_name) {
+  // Force a deep copy on platforms with reference counted strings.
+  string normalized_table_name(table_name.data(), table_name.size());
+  if (hms::HmsCatalog::IsEnabled()) {
+    // If HmsCatalog::NormalizeTableName returns an error, the table name is 
not
+    // modified. In this case the table is guaranteed to be a legacy table 
which
+    // has survived since before the cluster was configured to integrate with
+    // the HMS. It's safe to use the unmodified table name as the normalized
+    // name in this case, since there cannot be a name conflict with a table in
+    // the HMS. When the table gets 'upgraded' to be included in the HMS it 
will
+    // need to be renamed with a Hive compatible name.
+    //
+    // Note: not all legacy tables will fail normalization; if a table happens
+    // to be named with a Hive compatible name ("Legacy.Table"), it will be
+    // normalized according to the Hive rules ("legacy.table"). We check in
+    // TableLoader::VisitTables that such legacy tables do not have conflicting
+    // names when normalized.
+    ignore_result(hms::HmsCatalog::NormalizeTableName(&normalized_table_name));
+  }
+  return normalized_table_name;
+}
+
 const char* CatalogManager::StateToString(State state) {
   switch (state) {
     case CatalogManager::kConstructed: return "Constructed";

http://git-wip-us.apache.org/repos/asf/kudu/blob/7b048b8d/src/kudu/master/catalog_manager.h
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.h 
b/src/kudu/master/catalog_manager.h
index d27975a..3f13e38 100644
--- a/src/kudu/master/catalog_manager.h
+++ b/src/kudu/master/catalog_manager.h
@@ -683,6 +683,16 @@ class CatalogManager : public 
tserver::TabletReplicaLookupIf {
     return hms_catalog_.get();
   }
 
+  // Returns the normalized form of the provided table name.
+  //
+  // If the HMS integration is configured and the table name is a valid HMS
+  // table name, the normalized form is returned (see 
HmsCatalog::NormalizeTableName),
+  // otherwise a copy of the original table name is returned.
+  //
+  // If the HMS integration is not configured then a copy of the original table
+  // name is returned.
+  static std::string NormalizeTableName(const std::string& table_name);
+
  private:
   // These tests call ElectedAsLeaderCb() directly.
   FRIEND_TEST(MasterTest, TestShutdownDuringTableVisit);
@@ -753,9 +763,8 @@ class CatalogManager : public 
tserver::TabletReplicaLookupIf {
   // of authn tokens.
   Status PrepareFollowerTokenVerifier();
 
-  // Clears out the existing metadata ('table_names_map_', 'table_ids_map_',
-  // and 'tablet_map_'), loads tables metadata into memory and if successful
-  // loads the tablets metadata.
+  // Clears out the existing metadata (by-name map, table-id map, and tablet
+  // map), and loads table and tablet metadata into memory.
   Status VisitTablesAndTabletsUnlocked();
   // This is called by tests only.
   Status VisitTablesAndTablets();
@@ -953,26 +962,25 @@ class CatalogManager : public 
tserver::TabletReplicaLookupIf {
   typedef rw_spinlock LockType;
   mutable LockType lock_;
 
-  // Table maps: table-id -> TableInfo and table-name -> TableInfo
+  // Table maps: table-id -> TableInfo and normalized-table-name -> TableInfo
   TableInfoMap table_ids_map_;
-  TableInfoMap table_names_map_;
+  TableInfoMap normalized_table_names_map_;
 
   // Tablet maps: tablet-id -> TabletInfo
   TabletInfoMap tablet_map_;
 
-  // Names of tables that are currently reserved by CreateTable() or
-  // AlterTable().
+  // Names of tables that are currently reserved by CreateTable() or 
AlterTable().
   //
   // As a rule, operations that add new table names should do so as follows:
   // 1. Acquire lock_.
-  // 2. Ensure table_names_map_ does not contain the new name.
-  // 3. Ensure reserved_table_names_ does not contain the new name.
-  // 4. Add the new name to reserved_table_names_.
+  // 2. Ensure normalized_table_names_map_ does not contain the new normalized 
name.
+  // 3. Ensure reserved_normalized_table_names_ does not contain the new 
normalized name.
+  // 4. Add the new normalized name to reserved_normalized_table_names_.
   // 5. Release lock_.
   // 6. Perform the operation.
-  // 7. If it succeeded, add the name to table_names_map_ with lock_ held.
-  // 8. Remove the new name from reserved_table_names_ with lock_ held.
-  std::unordered_set<std::string> reserved_table_names_;
+  // 7. If it succeeded, add the normalized name to 
normalized_table_names_map_ with lock_ held.
+  // 8. Remove the new normalized name from reserved_normalized_table_names_ 
with lock_ held.
+  std::unordered_set<std::string> reserved_normalized_table_names_;
 
   Master *master_;
   ObjectIdGenerator oid_generator_;

http://git-wip-us.apache.org/repos/asf/kudu/blob/7b048b8d/src/kudu/mini-cluster/external_mini_cluster.cc
----------------------------------------------------------------------
diff --git a/src/kudu/mini-cluster/external_mini_cluster.cc 
b/src/kudu/mini-cluster/external_mini_cluster.cc
index 7db8edb..a188ad5 100644
--- a/src/kudu/mini-cluster/external_mini_cluster.cc
+++ b/src/kudu/mini-cluster/external_mini_cluster.cc
@@ -37,6 +37,7 @@
 #include "kudu/common/wire_protocol.pb.h"
 #include "kudu/gutil/basictypes.h"
 #include "kudu/gutil/strings/join.h"
+#include "kudu/gutil/strings/stringpiece.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/gutil/strings/util.h"
 #include "kudu/hms/mini_hms.h"
@@ -260,6 +261,20 @@ void ExternalMiniCluster::EnableMetastoreIntegration() {
   opts_.hms_mode = HmsMode::ENABLE_METASTORE_INTEGRATION;
 }
 
+void ExternalMiniCluster::DisableMetastoreIntegration() {
+  for (const auto& master : masters_) {
+    CHECK(master->IsShutdown()) << "Call Shutdown() before changing the HMS 
mode";
+    master->mutable_flags()->erase(
+        std::remove_if(
+          master->mutable_flags()->begin(), master->mutable_flags()->end(),
+          [] (const string& flag) {
+            return StringPiece(flag).starts_with("--hive_metastore");
+          }),
+        master->mutable_flags()->end());
+  }
+  opts_.hms_mode = HmsMode::ENABLE_HIVE_METASTORE;
+}
+
 void ExternalMiniCluster::SetDaemonBinPath(string daemon_bin_path) {
   opts_.daemon_bin_path = std::move(daemon_bin_path);
   for (auto& master : masters_) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/7b048b8d/src/kudu/mini-cluster/external_mini_cluster.h
----------------------------------------------------------------------
diff --git a/src/kudu/mini-cluster/external_mini_cluster.h 
b/src/kudu/mini-cluster/external_mini_cluster.h
index 15c9b02..7b29e00 100644
--- a/src/kudu/mini-cluster/external_mini_cluster.h
+++ b/src/kudu/mini-cluster/external_mini_cluster.h
@@ -320,9 +320,15 @@ class ExternalMiniCluster : public MiniCluster {
                  const std::string& value) WARN_UNUSED_RESULT;
 
   // Enable Hive Metastore integration.
-  // Overrides 'enable_metastore_integration' set by 
ExternalMiniClusterOptions.
+  // Overrides HMS integration options set by ExternalMiniClusterOptions.
+  // The cluster must be shut down before calling this method.
   void EnableMetastoreIntegration();
 
+  // Disable Hive Metastore integration.
+  // Overrides HMS integration options set by ExternalMiniClusterOptions.
+  // The cluster must be shut down before calling this method.
+  void DisableMetastoreIntegration();
+
   // Set the path where daemon binaries can be found.
   // Overrides 'daemon_bin_path' set by ExternalMiniClusterOptions.
   // The cluster must be shut down before calling this method.

Reply via email to