Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9788 )
Change subject: IMPALA-6719: Reset metadata database name case sensitivity ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/9788/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: http://gerrit.cloudera.org:8080/#/c/9788/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@981 PS3, Line 981: Db db = dbCache_.get().get(dbName); Instead of doing lowerCase() above, I think we should use getDb() here which already does the lower casing. The bug is that we are asking the dbCache_ directly instead of doing through the getDb() accessor. http://gerrit.cloudera.org:8080/#/c/9788/3/testdata/workloads/functional-query/queries/QueryTest/reset-metadata-case-sensitivity.test File testdata/workloads/functional-query/queries/QueryTest/reset-metadata-case-sensitivity.test: http://gerrit.cloudera.org:8080/#/c/9788/3/testdata/workloads/functional-query/queries/QueryTest/reset-metadata-case-sensitivity.test@3 PS3, Line 3: create database $DATABASE_FOO; Cleanup will not happen automatically if anything in this test fails. Our unique database fixture allows creating several unique databases that are cleaned up. Here's the relevant documentation from conftest.py: @UniqueDatabase.parametrize(name_prefix='mydb', num_dbs=3, sync_ddl=True) def test_something(self, vector, unique_database): # fixture creates databases mydb_48A80F, mydb_48A80F2, mydb_48A80F3 with sync_ddl self.client.execute('DROP TABLE IF EXISTS `{0}`.`mytable`'.format(unique_database)) # test does other stuff with the unique_database name as needed An example use can be found in test_ddl.py @UniqueDatabase.parametrize(sync_ddl=True, num_dbs=2) def test_alter_table(self, vector, unique_database): http://gerrit.cloudera.org:8080/#/c/9788/3/testdata/workloads/functional-query/queries/QueryTest/reset-metadata-case-sensitivity.test@6 PS3, Line 6: ==== Seems easier and more concise to write this as a loop in a python test. There are 3 different statements and we could randomly flip the casing of some characters, or try lower and upper case, or similar. http://gerrit.cloudera.org:8080/#/c/9788/3/tests/metadata/test_reset_metadata.py File tests/metadata/test_reset_metadata.py: http://gerrit.cloudera.org:8080/#/c/9788/3/tests/metadata/test_reset_metadata.py@22 PS3, Line 22: @UniqueDatabase.parametrize(sync_ddl=True) Why sync_ddl=True? -- To view, visit http://gerrit.cloudera.org:8080/9788 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id880aa559cec0afe8fbb7d33ccce83f7b5e474cb Gerrit-Change-Number: 9788 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Thu, 29 Mar 2018 22:37:29 +0000 Gerrit-HasComments: Yes
