impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4795: Allow fetching function obj from catalog using signature
Date Wed, 26 Jul 2017 18:59:11 GMT
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)
File fe/src/main/java/org/apache/impala/catalog/

Line 360:       throw new IllegalStateException("Expected function type to be either UDA or
> 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 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_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.


    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
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I2cfad0213a79d39b77ad9aff701a93f93be4bf7f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig <>
Gerrit-Reviewer: Bharath Vissapragada <>
Gerrit-Reviewer: Bikramjeet Vig <>
Gerrit-Reviewer: Matthew Jacobs <>
Gerrit-HasComments: Yes

View raw message