Skip to content

[SPARK-11191] [SQL] [FOLLOW-UP] Cleans up unnecessary anonymous HiveFunctionRegistry #9737

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

liancheng
Copy link
Contributor

According to discussion in PR #9664, the anonymous HiveFunctionRegistry in HiveContext can be removed now.

@SparkQA
Copy link

SparkQA commented Nov 16, 2015

Test build #45997 has finished for PR 9737 at commit 1cb40df.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@yhuai
Copy link
Contributor

yhuai commented Nov 16, 2015

Should we just pass in executionHive? We do not use other fields of HiveContext, right?

@liancheng
Copy link
Contributor Author

@yhuai I think metadataHive is also required if we want to handle persisted functions correctly later.

@yhuai
Copy link
Contributor

yhuai commented Nov 17, 2015

Then, should we add that later? I am thinking passing in HiveContext may be not really necessary if we only to use one or two fields.

@liancheng
Copy link
Contributor Author

Discussed with @yhuai offline. His major concerns are:

  1. Currently there's are circular references between HiveContext and HiveFunctionRegistry, which isn't obviously harmful for now, but would be good to eliminate.
  2. It's not necessary to pass the whole HiveContext into HiveFunctionRegistry when we only access 1 or 2 fields.

I agree. Updating.

@yhuai
Copy link
Contributor

yhuai commented Nov 17, 2015

LGTM pending jenkins.

@SparkQA
Copy link

SparkQA commented Nov 17, 2015

Test build #46058 has finished for PR 9737 at commit cd2b958.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@asfgit asfgit closed this in fa13301 Nov 17, 2015
@liancheng liancheng deleted the spark-11191.follow-up branch November 17, 2015 10:17
asfgit pushed a commit that referenced this pull request Nov 17, 2015
…ctionRegistry

According to discussion in PR #9664, the anonymous `HiveFunctionRegistry` in `HiveContext` can be removed now.

Author: Cheng Lian <lian@databricks.com>

Closes #9737 from liancheng/spark-11191.follow-up.

(cherry picked from commit fa13301)
Signed-off-by: Cheng Lian <lian@databricks.com>
@liancheng
Copy link
Contributor Author

Merged to master and cherry-picked to branch-1.5.

@liancheng
Copy link
Contributor Author

Also backported to branch-1.6.

asfgit pushed a commit that referenced this pull request Nov 17, 2015
…ctionRegistry

According to discussion in PR #9664, the anonymous `HiveFunctionRegistry` in `HiveContext` can be removed now.

Author: Cheng Lian <lian@databricks.com>

Closes #9737 from liancheng/spark-11191.follow-up.

(cherry picked from commit fa13301)
Signed-off-by: Cheng Lian <lian@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants