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 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/9788/4/tests/metadata/test_reset_metadata.py File tests/metadata/test_reset_metadata.py: http://gerrit.cloudera.org:8080/#/c/9788/4/tests/metadata/test_reset_metadata.py@28 PS4, Line 28: self.client.execute('drop database if exists %s cascade' % db) Why are we testing CREATE DATABASE/TABLE in here? I don't think we should test here that those statements lower case the db/table name. In that sense, I think a single database and table is sufficient. http://gerrit.cloudera.org:8080/#/c/9788/4/tests/metadata/test_reset_metadata.py@32 PS4, Line 32: for d in [db, db.lower(), db.upper()]: Aren't these extra loops redundant? This test is pretty expensive for covering a simple issue. http://gerrit.cloudera.org:8080/#/c/9788/4/tests/metadata/test_reset_metadata.py@34 PS4, Line 34: self.client.execute('refresh %s.%s' % (d, t)) Why not % (db, table) directly? I don't think we need to test that create database or create table -- 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: 4 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: Fri, 30 Mar 2018 04:45:02 +0000 Gerrit-HasComments: Yes
