Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9788 )

Change subject: IMPALA-6719: Reset metadata database name case sensitivity
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9788/1/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
File fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java:

http://gerrit.cloudera.org:8080/#/c/9788/1/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@60
PS1, Line 60: db.toLowerCase()
that'll change the toSql output. if that's not tested, pls add. the behavior 
here should not differ from the handling in TableName. fwict, the original 
string is retained (you see it in toString, toPath, flows around to thrift, 
etc.) but "equals()" does a "toLowerCase()".

I'm unclear what the impact is if the db name by itself is not handled in the 
same way as TableName. Ideally, we wrap it with something comparable (DbName?), 
but that may be more invasive than needed.


http://gerrit.cloudera.org:8080/#/c/9788/1/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/1/testdata/workloads/functional-query/queries/QueryTest/reset-metadata-case-sensitivity.test@2
PS1, Line 2: ---- QUERY
why not do these tests as a java unit test? FrontendTestBase has: addTestDb, 
addTestTable. You could then loop over case variations for db/table on 
create/refresh/invalidate as opposed to unrolling it  explicitly here.



--
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: 1
Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com>
Gerrit-Comment-Date: Mon, 26 Mar 2018 17:08:11 +0000
Gerrit-HasComments: Yes

Reply via email to