Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11279 )
Change subject: IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements ...................................................................... Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/11279/3/fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java File fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java: http://gerrit.cloudera.org:8080/#/c/11279/3/fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java@43 PS3, Line 43: analyzer.getDb(dbName_, Privilege.ALL, true, true); add /* parameter name */ before these bools to make the call more readable. right now, its unclear how the GRANT OPTION is specified. http://gerrit.cloudera.org:8080/#/c/11279/3/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/11279/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2472 PS3, Line 2472: /** I see its duplicated in three places now: method before, here, and next method-- this all can go stale easily. would be good to have the detail once and explain the others as just setting defaults for the more general method. http://gerrit.cloudera.org:8080/#/c/11279/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2636 PS3, Line 2636: Registers a table-level privilege request and an access event for auditing : * for the given table and privilege. The table must be a base table or a : * catalog view (not a local view). This method does not require the grant : * opti remove similar redundancy here with the method on L2651. http://gerrit.cloudera.org:8080/#/c/11279/3/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java File fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java: http://gerrit.cloudera.org:8080/#/c/11279/3/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java@38 PS3, Line 38: boolean grantOption_; explicitly set to false for default? http://gerrit.cloudera.org:8080/#/c/11279/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java: http://gerrit.cloudera.org:8080/#/c/11279/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@3785 PS3, Line 3785: testNumberOfMembers(TableRef.class, 21); why did this change? -- To view, visit http://gerrit.cloudera.org:8080/11279 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2485933c02b5384950b7c882ba1eb0fd703db5a3 Gerrit-Change-Number: 11279 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Adam Holley <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Wed, 22 Aug 2018 17:27:22 +0000 Gerrit-HasComments: Yes
