Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/10817 )
Change subject: WIP: KUDU-2191: support table-name identifiers with upper case chars ...................................................................... Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/10817/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10817/1//COMMIT_MSG@7 PS1, Line 7: with upper case chars > I feel it might be a bit confusing to call it support upper case table name I've added a 'summary' sentence which hopefully clarifies. http://gerrit.cloudera.org:8080/#/c/10817/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10817/2//COMMIT_MSG@47 PS2, Line 47: FOO.baz > nit: FOO.bar Done http://gerrit.cloudera.org:8080/#/c/10817/3/src/kudu/integration-tests/master_hms-itest.cc File src/kudu/integration-tests/master_hms-itest.cc: http://gerrit.cloudera.org:8080/#/c/10817/3/src/kudu/integration-tests/master_hms-itest.cc@65 PS3, Line 65: MasterHmsTest : public ExternalMiniClusterITestBase { > Should we add some more test cases with upper cases table name in other int Done http://gerrit.cloudera.org:8080/#/c/10817/3/src/kudu/integration-tests/master_hms-itest.cc@529 PS3, Line 529: cluster_->ShutdownNodes(cluster::ClusterNodes::MASTERS_ONLY); : cluster_->EnableMetastoreIntegration(); : ASSERT_TRUE(cluster_->Restart().IsNetworkError()); > nit: add a quick comment for what those lines are doing. Done http://gerrit.cloudera.org:8080/#/c/10817/3/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/10817/3/src/kudu/master/catalog_manager.cc@1488 PS3, Line 1488: if (FLAGS_catalog_manager_check_ts_count_for_create_table && > remove this, it was just to reduce log spam in tests. Done http://gerrit.cloudera.org:8080/#/c/10817/3/src/kudu/master/catalog_manager.cc@1513 PS3, Line 1513: table_name > When HMS integration is enabled, maybe we should log the normalized table It ends up being difficult to determine when the duplicate table only differs based on case, because the table object we lookup doesn't include the name. To get the name you have to take a CoW lock. Mind if we punt on this and revisit if experience shows it's a point of confusion? -- To view, visit http://gerrit.cloudera.org:8080/10817 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18977d6fe7b2999a36681a728ac0d1e54b7f38cd Gerrit-Change-Number: 10817 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Wed, 27 Jun 2018 17:41:13 +0000 Gerrit-HasComments: Yes
