Vihang Karajgaonkar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14121 )

Change subject: IMPALA-8851: Do not throw authorization exception in drop if 
exists queries
......................................................................


Patch Set 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/14121/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:

http://gerrit.cloudera.org:8080/#/c/14121/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2659
PS7, Line 2659: getFqTableName
I would have preferred a precondition check here instead of assuming what the 
caller wants since based on the result of this call, authorization decisions 
are being made.


http://gerrit.cloudera.org:8080/#/c/14121/6/fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
File fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java:

http://gerrit.cloudera.org:8080/#/c/14121/6/fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java@117
PS6, Line 117: tableName_
> I created a fix inside tableExists().
Sorry for bugging you on this. Adding the check in the tableExists won't be 
right if dbName_ here is not default. For instance if dbName_ = foo and 
tblName_.tblName = testTbl then this call will look up default.testTbl

Also, does the case matter. I think it is important to make sure that this call 
is doing the right thing otherwise a non-privileged user can drop the table.



--
To view, visit http://gerrit.cloudera.org:8080/14121
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iba068935e5da92d71e16e2321afdb8e7b781086a
Gerrit-Change-Number: 14121
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]>
Gerrit-Comment-Date: Tue, 27 Aug 2019 22:33:22 +0000
Gerrit-HasComments: Yes

Reply via email to