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 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/14121/6/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/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2659 PS6, Line 2659: tblName I was trying out your patch locally and I found a bug in this code which can lead to a privilege escalation problem. If the tblName is not fully qualified in this method this will always return false. Can you add a Precondition check to make sure that tblname is fully qualified here? Not sure why it was not caught in the tests. 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@115 PS6, Line 115: dbName_, getTbl() > I think this logic is faulty and will fail in the following scenario. Synced up with Csaba offline. If the user2 has privileges on t2 within the db the show tables command skips listing of t1. Hence a ANY privilege on db is not a suitable representative to know whether user has permissions to see the existance of t1. In such a case, the current approach makes sense to me. Thanks for clarification. http://gerrit.cloudera.org:8080/#/c/14121/6/fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java@117 PS6, Line 117: tableName_ I noticed that the tableName_ here is not fully qualified for some reason. We should pass TableName(dbName, getTbl()) here. -- 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: 6 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 21:10:43 +0000 Gerrit-HasComments: Yes
