Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10966 )
Change subject: IMPALA-7217: Incorrect UPDATE/DELETE authorization privilege ...................................................................... Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/10966/4/fe/src/main/java/org/apache/impala/analysis/FromClause.java File fe/src/main/java/org/apache/impala/analysis/FromClause.java: http://gerrit.cloudera.org:8080/#/c/10966/4/fe/src/main/java/org/apache/impala/analysis/FromClause.java@70 PS4, Line 70: registers clearer: Analyze this statement after registering the specified privileges. follow-up question here: would a map of table-refs to privs be useful for cases such as the delete test? the map can be used as an override, e.g., 'privilege' is applied unless there's an override. however, given the usage in ModifyStmt, perhaps targeting the specific table in the operation as suggested seems easier/clearer. http://gerrit.cloudera.org:8080/#/c/10966/4/fe/src/main/java/org/apache/impala/analysis/TableRef.java File fe/src/main/java/org/apache/impala/analysis/TableRef.java: http://gerrit.cloudera.org:8080/#/c/10966/4/fe/src/main/java/org/apache/impala/analysis/TableRef.java@629 PS4, Line 629: public void setPrivilege(Privilege privilege) { this.priv_ = privilege; } allowed to be null? -- To view, visit http://gerrit.cloudera.org:8080/10966 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I69d451f727a7df6c41166a15cf1ed6f5334dc739 Gerrit-Change-Number: 10966 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Adam Holley <ahol...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Fri, 03 Aug 2018 17:34:32 +0000 Gerrit-HasComments: Yes