impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dimitris Tsirogiannis (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-5259: Add REFRESH FUNCTIONS <db> statement
Date Sat, 20 May 2017 04:34:44 GMT
Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-5259: Add REFRESH FUNCTIONS <db> statement

Patch Set 7:

File fe/src/main/java/org/apache/impala/analysis/

PS4, Line 38: invalidating the entire catalog
"or refreshing database functions."

PS4, Line 110: toSql()
Don't we need to update this function?
File fe/src/main/java/org/apache/impala/catalog/

PS7, Line 628: loadFunctionsFromDbParams(tmpDb, msDb);
             :       // Load Java UDFs from HMS into the temporary db.
             :       loadJavaFunctions(tmpDb, javaFns);
Both these methods not only extract and load the functions into the target database but they
also increment and acquire a new catalog version for each function. This is redundant and
is actually performed again in L661. We should avoid this.

PS7, Line 635: catalogLock_.writeLock().lock();
This lock doesn't protect against concurrent operations on databases, hence the block in L636-670
is not safe. If you look at the CatalogOpExecutor, add/drop db/functions are protected by
the metastoreDdlLock_ (this lock variable is poorly named) and I believe this lock should
be used here instead.
File fe/src/main/java/org/apache/impala/catalog/

PS7, Line 256: // Remove the function first, in case there is an existing function with the
             :         // name and signature.
             :         removeFunction(catalogObject.getFn(), catalogObject.getCatalog_version());
This is weird and not expected. Is this because addFunction doesn't replace the function if
it already exists? If so, we should fix that and remove the removeFunction call.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I3625c88bb51cca833f3293c224d3f0feb00e6e0b
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Bharath Vissapragada <>
Gerrit-Reviewer: Dimitris Tsirogiannis <>
Gerrit-Reviewer: Taras Bobrovytsky <>
Gerrit-HasComments: Yes

View raw message