Taras Bobrovytsky has posted comments on this change.

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


Patch Set 9:

(5 comments)

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

PS9, Line 631: } catch (Exception e) {
             :       throw new CatalogException("Error refreshing functions in 
" + dbName + ": ", e);
             :     }
> You have two try-catch blocks with the same Exception type and the same act
Done


PS9, Line 648: fn.setCatalogVersion(incrementAndGetCatalogVersion());
> Is this needed?
Yes, if we don't increment it, the function won't be removed in Impala first. 
(Remember how we remove the function before adding it into Impalad catalog).


PS9, Line 662: // We do not need to increment and acquire a new catalog version 
for this
             :           // function here because this already happens when the 
functions are loaded
             :           // into tmpDb.
> nit: move comment above L660.
Done


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

PS9, Line 368: /**
             :    * Remove all functions.
             :    */
> remove
Done


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

PS9, Line 258: catalogObject.getCatalog_version()
Wanted to double check this. We don't have to do something special like in 
removeCatalogObject:

long dropCatalogVersion = catalogObject.getCatalog_version() == 0 ?
 currentCatalogUpdateVersion : catalogObject.getCatalog_version();

I don't think so, but wanted to double check.


-- 
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: 9
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