Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/15653 )
Change subject: IMPALA-9602: Fix case-sensitivity for local catalog ...................................................................... Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/15653/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15653/3//COMMIT_MSG@21 PS3, Line 21: start Impala with local catalog enabled : start-impala-cluster.py : --catalogd_args="--catalog_topic_mode=minimal" : --impalad_args="--use_local_catalog=true" : Create database in lower-case: "CREATE DATABASE db1;" : Run the following a few times (this errors without the patch): : impala-shell.sh -q "DROP TABLE IF EXISTS DB1.ddl_test1 PURGE; : CREATE TABLE DB1.ddl_test1 (val string) > We have a test_local_catalog.py or test_ddl.py in case you want to add this That's good to know about the python script which seems great for end-to-end test and testing race conditions in cache updates. I will keep this in mind for future testing. In this case though, since the problem is simpler, I am thinking the unit test is good enough. Let me know if you think otherwise. http://gerrit.cloudera.org:8080/#/c/15653/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java: http://gerrit.cloudera.org:8080/#/c/15653/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@625 PS3, Line 625: DbCacheKey > May be its better to move toLowerCase() call inside DbCacheKey constructor. I had thought of that but looking at existing code in other places (e.g Catalog.java, CatalogObjectCache.java), the caller explicitly lowercases instead of the constructor doing it. One motivation for it is there's a caseInsensitiveKeys_ flag in CatalogObjectCache to do this conditionally. I am thinking it would be good to maintain similar logic in the local catalog in case in the future we want to do this conditionally for any reason. What do you think ? -- To view, visit http://gerrit.cloudera.org:8080/15653 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3f368fa9b50e22ec5057d0bf66c3fd51064d4c26 Gerrit-Change-Number: 15653 Gerrit-PatchSet: 3 Gerrit-Owner: Aman Sinha <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]> Gerrit-Comment-Date: Wed, 08 Apr 2020 01:31:25 +0000 Gerrit-HasComments: Yes
