You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@impala.apache.org by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org> on 2016/06/07 20:03:18 UTC

[Impala-CR](cdh5-trunk) IMPALA-2836: ADD Partition now only requires INSERT permissions 

Bikramjeet Vig has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/3327

Change subject: IMPALA-2836: ADD Partition now only requires INSERT permissions
......................................................................

IMPALA-2836: ADD Partition now only requires INSERT permissions

Changed Permissions required in the analyse method in
AlterTableAddPartitionStmt along with changes in AlterTableStmt
file.

Testing:
Added a test in AuthorizationTest.java that runs an add partition
query without having Alter Permissions on that table.
Also removed a test that expected an error if trying to
achieve the same.

Change-Id: I6d971f0ab46fed87b0428d344d002a42644b9847
---
M fe/src/main/java/com/cloudera/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/com/cloudera/impala/analysis/AlterTableStmt.java
M fe/src/test/java/com/cloudera/impala/analysis/AuthorizationTest.java
3 files changed, 8 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/27/3327/1
-- 
To view, visit http://gerrit.cloudera.org:8080/3327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6d971f0ab46fed87b0428d344d002a42644b9847
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>

[Impala-CR](cdh5-trunk) IMPALA-2836: Change permissions for ADD/DROP PARTITION

Posted by "Dimitris Tsirogiannis (Code Review)" <ge...@cloudera.org>.
Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-2836: Change permissions for ADD/DROP PARTITION
......................................................................


Patch Set 4:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/3327/4//COMMIT_MSG
Commit Message:

PS4, Line 15: ALTER(Ownership)
Why is that the case? Shouldn't it be INSERT on table and ALL permission on the URI? Isn't that what Hive does?


http://gerrit.cloudera.org:8080/#/c/3327/4/fe/src/main/java/com/cloudera/impala/analysis/AlterTableStmt.java
File fe/src/main/java/com/cloudera/impala/analysis/AlterTableStmt.java:

Line 74:    * caller to specify privilege required for executing the alter statement
"the table"


Line 75:    * 
remove extra space


PS4, Line 76: @param analyzer
            :    * @param privilege
            :    *          the privilege required for the statement
            :    * @throws AnalysisException
We don't use java doc (for the most part). Can you plz remove this?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6d971f0ab46fed87b0428d344d002a42644b9847
Gerrit-PatchSet: 4
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-CR](cdh5-trunk) IMPALA-2836: ADD Partition now only requires INSERT permissions

Posted by "Dimitris Tsirogiannis (Code Review)" <ge...@cloudera.org>.
Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-2836: ADD Partition now only requires INSERT permissions
......................................................................


Patch Set 1:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/3327/1//COMMIT_MSG
Commit Message:

PS1, Line 7: ADD Partition
How about DROP or any other alter table statements? Does your change handle these cases as well? Shouldn't we have a consistent behavior across all alter table statements in terms of required permissions?


PS1, Line 9: analyse
analyze (typo)


PS1, Line 14: add partition
ADD PARTITION


PS1, Line 15: Permissions
permissions


PS1, Line 15: Alter
In general, you don't need to comment on every test case you added. Simply mentioning that you added/modified the authorization tests is sufficient.


http://gerrit.cloudera.org:8080/#/c/3327/1/fe/src/main/java/com/cloudera/impala/analysis/AlterTableStmt.java
File fe/src/main/java/com/cloudera/impala/analysis/AlterTableStmt.java:

PS1, Line 71:   
Extra space, please remove.


PS1, Line 72: protected void analyzeUsingPrivilege(Analyzer analyzer, Privilege privilege) throws AnalysisException {
Long line (lines should be less than 90 characters)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6d971f0ab46fed87b0428d344d002a42644b9847
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-CR](cdh5-trunk) IMPALA-2836: Change permissions for ADD/DROP PARTITION

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has uploaded a new patch set (#5).

Change subject: IMPALA-2836: Change permissions for ADD/DROP PARTITION
......................................................................

IMPALA-2836: Change permissions for ADD/DROP PARTITION

Currently, to add or drop a partition the user needs ALTER(Ownership)
permission. As an improvement to this and to closely resemble how Hive
handles permissions, the following change was made:
ALTER TABLE ADD/DROP PARTITION statement can be executed by a user
having INSERT permission

Testing:
Added/modified corresponding tests in AuthorizationTest.java

Change-Id: I6d971f0ab46fed87b0428d344d002a42644b9847
---
M fe/src/main/java/com/cloudera/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/com/cloudera/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/com/cloudera/impala/analysis/AlterTableStmt.java
M fe/src/test/java/com/cloudera/impala/analysis/AuthorizationTest.java
4 files changed, 21 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/27/3327/5
-- 
To view, visit http://gerrit.cloudera.org:8080/3327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6d971f0ab46fed87b0428d344d002a42644b9847
Gerrit-PatchSet: 5
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>

[Impala-CR](cdh5-trunk) IMPALA-2836: Change permissions for ADD/DROP PARTITION

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has uploaded a new patch set (#3).

Change subject: IMPALA-2836: Change permissions for ADD/DROP PARTITION
......................................................................

IMPALA-2836: Change permissions for ADD/DROP PARTITION

Currently, to add or drop a partition the user needs ALTER(Ownership)
permission. As an improvement to this and to closely resemble how Hive
handles permissions, the following changes were made:
 - ALTER TABLE ADD/DROP PARTITION statement can be executed by a user
   having INSERT permission
 - ALTER TABLE ADD PARTITION SET LOCATION 'hdfs_path_of_directory'
   still requires ALTER(Ownership) permission

Testing:
Added/modified corresponding tests in AuthorizationTest.java

Change-Id: I6d971f0ab46fed87b0428d344d002a42644b9847
---
M fe/src/main/java/com/cloudera/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/com/cloudera/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/com/cloudera/impala/analysis/AlterTableStmt.java
M fe/src/test/java/com/cloudera/impala/analysis/AuthorizationTest.java
4 files changed, 18 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/27/3327/3
-- 
To view, visit http://gerrit.cloudera.org:8080/3327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6d971f0ab46fed87b0428d344d002a42644b9847
Gerrit-PatchSet: 3
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>

[Impala-CR](cdh5-trunk) IMPALA-2836: Change permissions for ADD/DROP PARTITION

Posted by "Dimitris Tsirogiannis (Code Review)" <ge...@cloudera.org>.
Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-2836: Change permissions for ADD/DROP PARTITION
......................................................................


Patch Set 5: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6d971f0ab46fed87b0428d344d002a42644b9847
Gerrit-PatchSet: 5
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: No

[Impala-CR](cdh5-trunk) IMPALA-2836: Change permissions for ADD/DROP PARTITION

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has uploaded a new patch set (#4).

Change subject: IMPALA-2836: Change permissions for ADD/DROP PARTITION
......................................................................

IMPALA-2836: Change permissions for ADD/DROP PARTITION

Currently, to add or drop a partition the user needs ALTER(Ownership)
permission. As an improvement to this and to closely resemble how Hive
handles permissions, the following changes were made:
 - ALTER TABLE ADD/DROP PARTITION statement can be executed by a user
   having INSERT permission
 - ALTER TABLE ADD PARTITION LOCATION 'hdfs_path_of_directory'
   still requires ALTER(Ownership) permission

Testing:
Added/modified corresponding tests in AuthorizationTest.java

Change-Id: I6d971f0ab46fed87b0428d344d002a42644b9847
---
M fe/src/main/java/com/cloudera/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/com/cloudera/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/com/cloudera/impala/analysis/AlterTableStmt.java
M fe/src/test/java/com/cloudera/impala/analysis/AuthorizationTest.java
4 files changed, 27 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/27/3327/4
-- 
To view, visit http://gerrit.cloudera.org:8080/3327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6d971f0ab46fed87b0428d344d002a42644b9847
Gerrit-PatchSet: 4
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>

[Impala-CR](cdh5-trunk) IMPALA-2836: Change permissions for ADD/DROP PARTITION

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change.

Change subject: IMPALA-2836: Change permissions for ADD/DROP PARTITION
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/3327/3//COMMIT_MSG
Commit Message:

PS3, Line 14: ALTER TABLE ADD PARTITION SET LOCATION 'hdfs_path_of_directory'
> There is ALTER TABLE ADD PARTITION LOCATION and ALTER TABLE (PARTITION SPEC
Done


http://gerrit.cloudera.org:8080/#/c/3327/3/fe/src/main/java/com/cloudera/impala/analysis/AlterTableAddPartitionStmt.java
File fe/src/main/java/com/cloudera/impala/analysis/AlterTableAddPartitionStmt.java:

PS3, Line 84: if (location_ != null) {
            :       partitionSpec_.setPrivilegeRequirement(Privilege.ALTER);
            :       location_.analyze(analyzer, Privilege.ALL, FsAction.READ_WRITE);
            :     } else {
            :       partitionSpec_.setPrivilegeRequirement(Privilege.INSERT);
            :     }
> This is confusing to me. Why does the partition spec get a different privil
As mentioned in the commit message, when the partition location is specified in the query, we require ALTER permissions


http://gerrit.cloudera.org:8080/#/c/3327/3/fe/src/main/java/com/cloudera/impala/analysis/AlterTableDropPartitionStmt.java
File fe/src/main/java/com/cloudera/impala/analysis/AlterTableDropPartitionStmt.java:

PS3, Line 73: INSERT
> Please correct me if I am wrong, but I thought the DROP will still require 
In order to closely resemble how hive handles permissions, this was done. Hive requires DELETE permissions for this but we do not support fine grained permissions and instead couple up DELETE permissions with INSERT.


http://gerrit.cloudera.org:8080/#/c/3327/3/fe/src/main/java/com/cloudera/impala/analysis/AlterTableStmt.java
File fe/src/main/java/com/cloudera/impala/analysis/AlterTableStmt.java:

PS3, Line 72: analyzeUsingPrivilege
> Let's keep the name "analyze" for the function. Also, you may want to add a
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6d971f0ab46fed87b0428d344d002a42644b9847
Gerrit-PatchSet: 3
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-CR](cdh5-trunk) IMPALA-2836: Change permissions for ADD/DROP PARTITION

Posted by "Dimitris Tsirogiannis (Code Review)" <ge...@cloudera.org>.
Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-2836: Change permissions for ADD/DROP PARTITION
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/3327/3//COMMIT_MSG
Commit Message:

PS3, Line 14: ALTER TABLE ADD PARTITION SET LOCATION 'hdfs_path_of_directory'
There is ALTER TABLE ADD PARTITION LOCATION and ALTER TABLE (PARTITION SPEC) SET LOCATION .... Which one do you mean?


http://gerrit.cloudera.org:8080/#/c/3327/3/fe/src/main/java/com/cloudera/impala/analysis/AlterTableAddPartitionStmt.java
File fe/src/main/java/com/cloudera/impala/analysis/AlterTableAddPartitionStmt.java:

PS3, Line 84: if (location_ != null) {
            :       partitionSpec_.setPrivilegeRequirement(Privilege.ALTER);
            :       location_.analyze(analyzer, Privilege.ALL, FsAction.READ_WRITE);
            :     } else {
            :       partitionSpec_.setPrivilegeRequirement(Privilege.INSERT);
            :     }
This is confusing to me. Why does the partition spec get a different privilege request depending on the existence of location?


http://gerrit.cloudera.org:8080/#/c/3327/3/fe/src/main/java/com/cloudera/impala/analysis/AlterTableDropPartitionStmt.java
File fe/src/main/java/com/cloudera/impala/analysis/AlterTableDropPartitionStmt.java:

PS3, Line 73: INSERT
Please correct me if I am wrong, but I thought the DROP will still require full permissions, no?


http://gerrit.cloudera.org:8080/#/c/3327/3/fe/src/main/java/com/cloudera/impala/analysis/AlterTableStmt.java
File fe/src/main/java/com/cloudera/impala/analysis/AlterTableStmt.java:

PS3, Line 72: analyzeUsingPrivilege
Let's keep the name "analyze" for the function. Also, you may want to add a function comment here and in the function above.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6d971f0ab46fed87b0428d344d002a42644b9847
Gerrit-PatchSet: 3
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-CR](cdh5-trunk) IMPALA-2836: Change permissions for ADD/DROP PARTITION

Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has abandoned this change.

Change subject: IMPALA-2836: Change permissions for ADD/DROP PARTITION
......................................................................


Abandoned

As discussed in the JIRA, the right fix is IMPALA-3840 and we should focus on that.

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I6d971f0ab46fed87b0428d344d002a42644b9847
Gerrit-PatchSet: 5
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>

[Impala-CR](cdh5-trunk) IMPALA-2836: Change permissions for ADD/DROP PARTITION

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change.

Change subject: IMPALA-2836: Change permissions for ADD/DROP PARTITION
......................................................................


Patch Set 4:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/3327/4//COMMIT_MSG
Commit Message:

PS4, Line 15: ALTER(Ownership)
> Why is that the case? Shouldn't it be INSERT on table and ALL permission on
Done


http://gerrit.cloudera.org:8080/#/c/3327/4/fe/src/main/java/com/cloudera/impala/analysis/AlterTableStmt.java
File fe/src/main/java/com/cloudera/impala/analysis/AlterTableStmt.java:

Line 74:    * caller to specify privilege required for executing the alter statement
> "the table"
Done


Line 75:    * 
> remove extra space
Done


PS4, Line 76: @param analyzer
            :    * @param privilege
            :    *          the privilege required for the statement
            :    * @throws AnalysisException
> We don't use java doc (for the most part). Can you plz remove this?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6d971f0ab46fed87b0428d344d002a42644b9847
Gerrit-PatchSet: 4
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-CR](cdh5-trunk) IMPALA-2836: Change permissions for ADD/DROP PARTITION

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has uploaded a new patch set (#2).

Change subject: IMPALA-2836: Change permissions for ADD/DROP PARTITION
......................................................................

IMPALA-2836: Change permissions for ADD/DROP PARTITION

Currently, to add or drop a partition the user needs ALTER(Ownership)
permission. As an improvement to this and to closely resemble how Hive
handles permissions, the following changes were made:
 - ALTER TABLE ADD/DROP PARTITION statement can be executed by a user
   having INSERT permission
 - ALTER TABLE ADD PARTITION SET LOCATION 'hdfs_path_of_directory'
   still requires ALTER(Ownership) permission

Testing:
Added/modified corresponding tests in AuthorizationTest.java

Change-Id: I6d971f0ab46fed87b0428d344d002a42644b9847
---
M fe/src/main/java/com/cloudera/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/com/cloudera/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/com/cloudera/impala/analysis/AlterTableStmt.java
M fe/src/test/java/com/cloudera/impala/analysis/AuthorizationTest.java
4 files changed, 19 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/27/3327/2
-- 
To view, visit http://gerrit.cloudera.org:8080/3327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6d971f0ab46fed87b0428d344d002a42644b9847
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>

[Impala-CR](cdh5-trunk) IMPALA-2836: ADD Partition now only requires INSERT permissions

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change.

Change subject: IMPALA-2836: ADD Partition now only requires INSERT permissions
......................................................................


Patch Set 1:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/3327/1//COMMIT_MSG
Commit Message:

PS1, Line 7: ADD Partition
> How about DROP or any other alter table statements? Does your change handle
Done


PS1, Line 9: analyse
> analyze (typo)
Done


PS1, Line 14: add partition
> ADD PARTITION
Done


PS1, Line 15: Permissions
> permissions
Done


PS1, Line 15: Alter
> In general, you don't need to comment on every test case you added. Simply 
Done


http://gerrit.cloudera.org:8080/#/c/3327/1/fe/src/main/java/com/cloudera/impala/analysis/AlterTableStmt.java
File fe/src/main/java/com/cloudera/impala/analysis/AlterTableStmt.java:

PS1, Line 71:   
> Extra space, please remove.
Done


PS1, Line 72: protected void analyzeUsingPrivilege(Analyzer analyzer, Privilege privilege) throws AnalysisException {
> Long line (lines should be less than 90 characters)
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6d971f0ab46fed87b0428d344d002a42644b9847
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: Yes