Taras Bobrovytsky has posted comments on this change.

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


Patch Set 7:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/6878/4/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
File fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java:

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


PS4, Line 110: toSql()
> Don't we need to update this function?
Done


http://gerrit.cloudera.org:8080/#/c/6878/7/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

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 
Done


PS7, Line 635: catalogLock_.writeLock().lock();
> This lock doesn't protect against concurrent operations on databases, hence
Done


http://gerrit.cloudera.org:8080/#/c/6878/7/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
File fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java:

PS7, Line 256: // Remove the function first, in case there is an existing 
function with the same
             :         // name and signature.
             :         removeFunction(catalogObject.getFn(), 
catalogObject.getCatalog_version());
> This is weird and not expected. Is this because addFunction doesn't replace
It doesn't replace it if its "indistinguishable". We don't check that the 
binary is the same or different.

How should we modify the addFunction call? Always remove the function there?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3625c88bb51cca833f3293c224d3f0feb00e6e0b
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky <tbobrovyt...@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to