Matthew Jacobs 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.");
> I believe IMPALA-626 is what caused the problem filed in IMPALA-4795 in the
You just described the issue in IMPALA-4795. I agree that IMPALA-626 introduced 
the code that caused Function.fromThrift to throw, but my point was about 
testing  the scenario described in IMPALA-626 (and unfortunately the JIRA text 
isn't very useful).

The code path I'm talking about is not what you mentioned, it's in 
impala-server.cc where we process topic deletions (l1359 for me). We know that 
IMPALA-626 caused the fn lookup to fail, but in the code I'm talking about that 
means the fn isn't being properly deleted. Now with your fix, the function will 
be deleted. Regardless of this exception here, IMPALA-626's scenario is worth 
testing. 

The test test_udfs.py test_drop_function_while_running almost simulates that, 
but doesn't add the function back again. Can you modify that test to add the fn 
back again? Of course please make sure all the tests pass.


l1359:

    for (const string& key: delta.topic_deletions) {
      LOG(INFO) << "Catalog topic entry deletion: " << key;
      TCatalogObject catalog_object;
      Status status = TCatalogObjectFromEntryKey(key, &catalog_object);


-- 
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 <bikramjeet....@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to