Skip to content

Conversation

@windpiger
Copy link
Contributor

What changes were proposed in this pull request?

when lookupfunction in HiveSessionCatalog, first look up in spark's build-in functionRegistry, if it not existed, then look up the Hive'S build-in functions which are listed in Seq(hiveFunctions).

But the [hash] function is already in spark's build-in functionRegistry list, so it will never to go to look up in Hive's hiveFunctions list, so the [hash] function which is in Hive's hiveFunctions is redundant, we can remove it.

@cloud-fan
Copy link
Contributor

ok to test

@cloud-fan
Copy link
Contributor

LGTM

@SparkQA
Copy link

SparkQA commented Nov 4, 2016

Test build #68138 has finished for PR 15766 at commit c3a0389.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@andrewor14
Copy link
Contributor

retest this please

@SparkQA
Copy link

SparkQA commented Nov 4, 2016

Test build #68152 has finished for PR 15766 at commit c3a0389.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@rxin
Copy link
Contributor

rxin commented Nov 4, 2016

Don't we actually unregister the Spark built-in one when testing Hive?

@cloud-fan
Copy link
Contributor

@rxin good catch! We do unregister the spark builtin hash in test: https://github.com/apache/spark/blob/master/sql/hive/compatibility/src/test/scala/org/apache/spark/sql/hive/execution/HiveCompatibilitySuite.scala#L60-L61

So we have a little more work to do here, we should register the hive hash function in that test suite.

@gatorsmile
Copy link
Member

I am working on the related issue in #14498

@windpiger
Copy link
Contributor Author

@rxin @cloud-fan you are rigth,hash should be unregistered and replace with Hive's hash, or we could put the failed hash testcase into blacklist as @gatorsmile 's work #14498 . I will close the PR

@windpiger
Copy link
Contributor Author

@cloud-fan I will appreciate that you can help to close this PR~

@cloud-fan
Copy link
Contributor

Only the owner(yourself) can close this PR.

@windpiger windpiger closed this Nov 7, 2016
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.

6 participants