[
https://issues.apache.org/jira/browse/HIVE-2779?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Phabricator updated HIVE-2779:
------------------------------
Attachment: HIVE-2779.D1599.1.patch
kevinwilfong requested code review of "HIVE-2779 [jira] Improve hooks run in
Driver".
Reviewers: JIRA
https://issues.apache.org/jira/browse/HIVE-2779
Merged the various methods in Driver to get hooks into a single method. I
took care to ensure that the checks that were occurring for SemanticAnalyzer
hooks, but not for other hooks, as they are not necessarily applicable.
While doing this, I also mentioned in the comments on the method that the
hooks are returned in the same order they were specified, and this was
intentional.
I also updated the HiveSemanticAnalyzerHookContext to include the inputs and
outputs from the SemanticAnalyzer before it is passed to any hooks which run
after analysis. I wrote it in a way such that it should not be difficult to
add more should we need them.
There are some small improvements that can be made to the hooks which are run
in the Driver:
1) The code to get hooks has been clearly just been copy+pasted for each of
Pre/Post/OnFailure/SemanticAnalyzer hooks. This code should be consolidated
into a single method.
2) There is a lot more information available to SemanticAnalyzer hooks which
ran after semantic analysis than to those that run before, such as inputs and
outputs. We should make some of this information available to those hooks,
preferably through HiveSemanticAnalyzerHookContext, so that existing hooks
aren't broken.
3) Currently, possibly unintentionally, hooks are initialized and run in the
order they appear in the comma separated list that is the value of the
configuration variable. This is a useful property, we should add comments
indicating this is desired and add a unit test to enforce it.
TEST PLAN
EMPTY
REVISION DETAIL
https://reviews.facebook.net/D1599
AFFECTED FILES
ql/src/test/results/clientnegative/bad_exec_hooks.q.out
ql/src/test/results/clientpositive/hook_order.q.out
ql/src/test/org/apache/hadoop/hive/ql/hooks/VerifyHooksRunInOrder.java
ql/src/test/queries/clientpositive/hook_order.q
ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveUtils.java
ql/src/java/org/apache/hadoop/hive/ql/parse/HiveSemanticAnalyzerHook.java
ql/src/java/org/apache/hadoop/hive/ql/parse/HiveSemanticAnalyzerHookContext.java
ql/src/java/org/apache/hadoop/hive/ql/parse/HiveSemanticAnalyzerHookContextImpl.java
ql/src/java/org/apache/hadoop/hive/ql/Driver.java
MANAGE HERALD DIFFERENTIAL RULES
https://reviews.facebook.net/herald/view/differential/
WHY DID I GET THIS EMAIL?
https://reviews.facebook.net/herald/transcript/3399/
Tip: use the X-Herald-Rules header to filter Herald messages in your client.
> Improve hooks run in Driver
> ---------------------------
>
> Key: HIVE-2779
> URL: https://issues.apache.org/jira/browse/HIVE-2779
> Project: Hive
> Issue Type: Improvement
> Reporter: Kevin Wilfong
> Assignee: Kevin Wilfong
> Attachments: HIVE-2779.D1599.1.patch
>
>
> There are some small improvements that can be made to the hooks which are run
> in the Driver:
> 1) The code to get hooks has been clearly just been copy+pasted for each of
> Pre/Post/OnFailure/SemanticAnalyzer hooks. This code should be consolidated
> into a single method.
> 2) There is a lot more information available to SemanticAnalyzer hooks which
> ran after semantic analysis than to those that run before, such as inputs and
> outputs. We should make some of this information available to those hooks,
> preferably through HiveSemanticAnalyzerHookContext, so that existing hooks
> aren't broken.
> 3) Currently, possibly unintentionally, hooks are initialized and run in the
> order they appear in the comma separated list that is the value of the
> configuration variable. This is a useful property, we should add comments
> indicating this is desired and add a unit test to enforce it.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
