Vuk Ercegovac has posted comments on this change. ( )

Change subject: IMPALA-5152: Introduce metadata loading phase

Patch Set 3:

File fe/src/main/java/org/apache/impala/analysis/
PS3, Line 460: // Process statements for which column-level privilege requests 
may be registered
             :     // except for DESCRIBE TABLE, REFRESH/INVALIDATE, USE or 
stale comment?
PS3, Line 462: isResult_.isQueryStmt() || analysisResult_.isInsertStmt() ||
             :         analysisResult_.isUpdateStmt() || 
analysisResult_.isDeleteStmt() ||
             :         analysi
worth grouping into a hasColumnPrivilege method?
PS3, Line 508: sult_.isDescribeTableStmt() ||
             :             analysisResult_.isResetMetadataStmt() ||
             :             analysisResult_.isUseStmt() ||
             :             analysisResult_.isShowTablesStmt() ||
             :             analysisResult_
what's special about these? how would one know that altertable belongs here?
File fe/src/main/java/org/apache/impala/analysis/
PS3, Line 309:     // the map was populated.
is that TODO about other catalog objects still relevant?
File fe/src/main/java/org/apache/impala/analysis/
PS3, Line 124: || expr.contains(
comment is stale, pls update.
File fe/src/main/java/org/apache/impala/analysis/
PS3, Line 62: tableName_.toPath()
add a precondition in the constructor to check that this is not null.
File fe/src/main/java/org/apache/impala/analysis/
PS3, Line 71: tableName_.toPath()
check not-null in constructor.
File fe/src/main/java/org/apache/impala/service/
PS3, Line 961: DatabaseNotFoundException
fwict, this is the exception that we were previously using to give back an 
error message about a non-existent db whereas now we state that the table is 
missing. would it be reasonable to record this in the table cache (in a 
separate map)?
PS3, Line 969: Tbls.put(tblName, t
so we can have views that are marked as loaded, but their base tables may not 
be loaded? not sure what we do for renaming a base table of a view.
File fe/src/test/java/org/apache/impala/analysis/
PS1, Line 104: Table does
> Fair point. We are somewhat inconsistent about this today. Here's some back
My guess is that the case you refer to is the common case. If the error is more 
specific, I think that's useful (e.g., db name typo). I saw in the frontend 
code where we get this info so if its not too cumbersome to carry around, would 
be useful. I'll note that the new error is technically correct so don't I feel 
too strongly about modifying the proposed behavior.

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I68d32d5acd4a6f6bc6cedb05e6cc5cf604d24a55
Gerrit-Change-Number: 8958
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Behm <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Bharath Vissapragada <>
Gerrit-Reviewer: Dimitris Tsirogiannis <>
Gerrit-Reviewer: Philip Zeyliger <>
Gerrit-Reviewer: Vuk Ercegovac <>
Gerrit-Comment-Date: Tue, 13 Feb 2018 01:13:58 +0000
Gerrit-HasComments: Yes

Reply via email to