Alex Behm has posted comments on this change.

Change subject: IMPALA-1654: General partition exprs in DDL operations.

Patch Set 11:


Amos, sorry to meddle with your patch but it seemed easier/faster this way 
since I have no idea what's wrong with your env.

I fixed the AuthorizationTest and AuditingTest for you. Please look at the 
changes carefully and try to apply the same principles in other places in your 

I added comments to explain what the bug was.

I'm still looking at those other failing tests. Hopefully we can fix your env 
so you can repro yourself.
File fe/src/main/java/com/cloudera/impala/analysis/

Line 70:   public void analyze(Analyzer analyzer) throws AnalysisException {
The changes here lead to a bunch of changes in error messages which I think are 

Can you fix the expected error messages in the FE tests, e.g., AnalyzeDDLTest?

Line 72:     TableRef tableRef = new TableRef(tableName_.toPath(), null, 
This version is better because:
* we only ask the catalog for this table once, avoiding potential inconsistency 
* we register the appropriate audit and auth events exactly once in 
* more streamlined code

Line 80:     if (tableRef instanceof CollectionTableRef) {
Could you add a test for this?
File fe/src/main/java/com/cloudera/impala/analysis/

Line 61:     TableRef tableRef = new TableRef(tableName_.toPath(), null, 
I'm explaining the auth/audit bug in the following comments.

Line 65:     if (!(table_ instanceof HdfsTable)) {
This registers audit and auth events for the VIEW_METADATA privilege.

Line 74:       partitionSet_.setPartitionShouldExist();
This registers audit and auth events for the SELECT privilege - which is wrong 
and caused tests to fail.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c9162fcf9d227b8daf4c2e761d57bab4e26408f
Gerrit-PatchSet: 11
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Amos Bird <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Amos Bird <>
Gerrit-HasComments: Yes

Reply via email to