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: (4 comments) Will fixup remaining comments with code changes. http://gerrit.cloudera.org:8080/#/c/10817/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10817/3//COMMIT_MSG@54 PS3, Line 54: source : of confusion > I feel confusion might be mitigated if we always store the normalized form, The main concern with storing normalized table names is that a client that issues CREATE TABLE FOO, then a ListTables RPC will receive [ "foo" ] and not ["FOO"]. I think the patch as implemented is overall less surprising given the existing Kudu semantics. http://gerrit.cloudera.org:8080/#/c/10817/3/src/kudu/hms/hms_catalog.cc File src/kudu/hms/hms_catalog.cc: http://gerrit.cloudera.org:8080/#/c/10817/3/src/kudu/hms/hms_catalog.cc@105 PS3, Line 105: '_', and '/' > Is this naming restriction configurable in HMS? The HMS appears to always allow underscores, and allows forward slashes by default, but can be configured not to allow forward slashes. When running against an HMS which is configured to disallow '/' the HMS should maintain the invariant so we don't need to do anything special. We could change this error message to be more generic like '... identifier pair, each which must be Hive compatible', but then our users will have to go and look up Hive identifier rules, and their docs are _awful_. HMS checks identifiers here: https://github.com/apache/hive/blob/master/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java#L486 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. http://gerrit.cloudera.org:8080/#/c/10817/3/src/kudu/master/catalog_manager.cc@2301 PS3, Line 2301: In this case > Since without HMS integration, we do not allow rename to same table name. I If we didn't allow it there would be no way to change the non-normalized name of a table, and the non-normalized name is user visible via the ListTables RPC. -- 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: Tue, 26 Jun 2018 23:52:04 +0000 Gerrit-HasComments: Yes
