Bikramjeet Vig has posted comments on this change. Change subject: IMPALA-4795: Allow fetching function obj from catalog using signature ......................................................................
Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/7479/1/fe/src/main/java/org/apache/impala/catalog/Function.java File fe/src/main/java/org/apache/impala/catalog/Function.java: Line 360: throw new IllegalStateException("Expected function type to be either UDA or UDF."); > This seems like a reasonable thing to do later, but I'm not sure if this wi I believe it makes sense to change the behavior over here for the following reasons: Function.toThrift is currently used at 3 other sites where if an exception is thrown then: 1. CatalogServiceCatalog:loadFunctionsFromDbParams -> used to load metadata from HMS and handles exception for each entry in the HMS. So, if one entry fails it wont affect the whole loading process. 2. ImpalaCatalog:addFunction -> used while processing TCatalogUpdateReq. Any exception thrown here will be caught in ImpaladCatalog:updateCatalog where it is handled for each entry in the TCatalogUpdateReq object. So wont affect the entire flow. 3. CatalogOpExecutor:createFunction -> this is used to create a new function object while processing a DDL request, so if an exception is thrown here then it will fail the request. Moreover, in first 2 cases if the TFunction object does not have scalar_fn or aggregate_fn field set and if we don't throw an exception, a generic Function object will be created which will eventually never be used since its neither a scalar nor an aggregate function. For the last case, we are only allowed to create a scalar or aggregate function using DDL request, so if these fields are not set then it definitely is a problem and we should be throwing an exception -- To view, visit http://gerrit.cloudera.org:8080/7479 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2cfad0213a79d39b77ad9aff701a93f93be4bf7f Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-HasComments: Yes
