Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/14867 )
Change subject: IMPALA-9222: Speed up show tables/DBs if the user has access to parent db/server ...................................................................... Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/14867/2/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/14867/2/fe/src/main/java/org/apache/impala/service/Frontend.java@777 PS2, Line 777: && !userHasDbLevelAccess(user, dbName)) { > What about passing dbName into userHasDbLevelAccess() and dealing with db b Done http://gerrit.cloudera.org:8080/#/c/14867/3/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/14867/3/fe/src/main/java/org/apache/impala/service/Frontend.java@967 PS3, Line 967: in > typo? is => in? Done http://gerrit.cloudera.org:8080/#/c/14867/3/fe/src/main/java/org/apache/impala/service/Frontend.java@975 PS3, Line 975: onDb > Relax this to onAnyColumn as used in isAccessibleToUser() above? Actually I did that at first and broke several tests. The two are intentionally different: - userHasDbLevelAccess(): the user has SELECT privilege on the whole database, which is inherited by every table (and column), so no individual checks are needed for the tables. I could have used more privileges than SELECT, e.g. CREATE also means that you have the right to list tables IMO, but this is more questionable, and I think the SELECT is enough for the optimization in most use cases, e.g. when the user is Admin or the owner of the database. - isAccessibleToUser() means that the user has any kind of access inside the database, even one column in a single table is enough. This only means that the user has the right to know about the existence of the database, e.g. call USE, see it in SHOW DATABASE. E.g. the default db is special, you can always USE it, but this doesn't mean that you have the right to know about the existence on every table - privileges on the given table or the whole db is needed for that. -- To view, visit http://gerrit.cloudera.org:8080/14867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic1f5c5d1cf447a9f1cec46c45272f250b8580826 Gerrit-Change-Number: 14867 Gerrit-PatchSet: 4 Gerrit-Owner: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Mon, 09 Dec 2019 12:50:51 +0000 Gerrit-HasComments: Yes
