[SPARK-14335][SQL] Describe function command returns wrong output by yongtang · Pull Request #12128 · apache/spark

yongtang · 2016-04-02T07:43:03Z

What changes were proposed in this pull request?

…because some of built-in functions are not in function registry.

This fix tries to fix issues in describe function command where some of the outputs
still shows Hive's function because some built-in functions are not in FunctionRegistry.

The following built-in functions have been added to FunctionRegistry:

-
!
*
/
&
%
^
+
<
<=
<=>
=
==
>
>=
|
~
and
in
like
not
or
rlike
when

The following listed functions are not added, but hard coded in commands.scala (@hvanhovell):

!=
<>
between
case

Below are the existing result of the above functions that have not been added:

spark-sql> describe function `!=`;
Function: <>
Class: org.apache.hadoop.hive.ql.udf.generic.GenericUDFOPNotEqual
Usage: a <> b - Returns TRUE if a is not equal to b

spark-sql> describe function `<>`;
Function: <>
Class: org.apache.hadoop.hive.ql.udf.generic.GenericUDFOPNotEqual
Usage: a <> b - Returns TRUE if a is not equal to b

spark-sql> describe function `between`;
Function: between
Class: org.apache.hadoop.hive.ql.udf.generic.GenericUDFBetween
Usage: between a [NOT] BETWEEN b AND c - evaluate if a is [not] in between b and c

spark-sql> describe function `case`;
Function: case
Class: org.apache.hadoop.hive.ql.udf.generic.GenericUDFCase
Usage: CASE a WHEN b THEN c [WHEN d THEN e]* [ELSE f] END - When a = b, returns c; when a = d, return e; else return f

How was this patch tested?

Existing tests passed. Additional test cases added.

SparkQA · 2016-04-02T22:06:34Z

Test build #2735 has finished for PR 12128 at commit 7a84e9b.

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

hvanhovell · 2016-04-03T18:23:17Z

@yhuai what do you want to do with the composed expressions, i.e.: !=, <> & between? case is more of a keyword.

yhuai · 2016-04-03T18:58:25Z

For composed expressions, if there are only a few of them, maybe we can have special handling in the describe command? For case, we do not need to support describe function case if it is more of a keyword.

yongtang · 2016-04-03T19:21:09Z

Hi @yhuai @hvanhovell , as a reference, below is the result of the describe function for current master. I haven't been able to identify the corresponding functions:

spark-sql> describe function `!=`;
Function: <>
Class: org.apache.hadoop.hive.ql.udf.generic.GenericUDFOPNotEqual
Usage: a <> b - Returns TRUE if a is not equal to b

spark-sql> describe function `<>`;
Function: <>
Class: org.apache.hadoop.hive.ql.udf.generic.GenericUDFOPNotEqual
Usage: a <> b - Returns TRUE if a is not equal to b

spark-sql> describe function `between`;
Function: between
Class: org.apache.hadoop.hive.ql.udf.generic.GenericUDFBetween
Usage: between a [NOT] BETWEEN b AND c - evaluate if a is [not] in between b and c

spark-sql> describe function `case`;
Function: case
Class: org.apache.hadoop.hive.ql.udf.generic.GenericUDFCase
Usage: CASE a WHEN b THEN c [WHEN d THEN e]* [ELSE f] END - When a = b, returns c; when a = d, return e; else return f

hvanhovell · 2016-04-03T20:45:25Z

@yongtang can you see if you can hardcode the descriptions for the composed function into the DecribeFunction command? See: https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/command/commands.scala#L421-L423

yongtang · 2016-04-03T21:03:07Z

@hvanhovell Thanks. Let me take a look and update the pull request.

yongtang · 2016-04-04T06:05:40Z

Hi @hvanhovell the pull request was updated. Let me know if there are any issues.

yhuai · 2016-04-04T06:08:49Z

ok to test

yhuai · 2016-04-04T06:14:45Z

sql/core/src/main/scala/org/apache/spark/sql/execution/command/commands.scala

Can we add the usage at here?

@yhuai Sure let me update the pull request shortly.

yongtang · 2016-04-04T06:43:44Z

Hi @yhuai the pull request has been updated with the usages being added. Let me know if there are any other issues. And thanks for the review.

SparkQA · 2016-04-04T07:28:08Z

Test build #54829 has finished for PR 12128 at commit baad78f.

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

SparkQA · 2016-04-04T08:00:04Z

Test build #54834 has finished for PR 12128 at commit f318e8c.

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

SparkQA · 2016-04-04T16:46:03Z

Test build #54855 has finished for PR 12128 at commit 6e1e879.

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

SparkQA · 2016-04-04T22:17:14Z

Test build #2743 has finished for PR 12128 at commit 6e1e879.

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

yhuai · 2016-04-04T23:19:49Z

sql/core/src/main/scala/org/apache/spark/sql/execution/command/commands.scala

Looks like we also need to match upper case letters?

Thanks @yhuai let me update the pull request.

yongtang · 2016-04-05T02:05:30Z

Thanks @yhuai. The pull request has been updated. Please let me know if there are any other issues.

SparkQA · 2016-04-05T02:15:45Z

Test build #54928 has finished for PR 12128 at commit 353a132.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds no public classes.

hvanhovell · 2016-04-05T06:33:33Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala

Could you also add a test case for one of the hard coded functions?

@hvanhovell Thanks. Additional test cases have been added.

SparkQA · 2016-04-05T15:55:21Z

Test build #54986 has finished for PR 12128 at commit 927272c.

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

yhuai · 2016-04-09T17:04:23Z

@yongtang This PR looks good to me. Can you rebase?

…ause some of built-in functions are not in function registry. This fix tries to fix issues in `describe function` command where some of the outputs still shows Hive's function because some built-in functions are not in FunctionRegistry. The following built-in functions have been added to FunctionRegistry: ``` - ! * / & % ^ + < <= <=> = == > >= | ~ and in like not or rlike when ``` The following listed functions are not added: ``` != <> between case ``` All existing tests passed.

@hvanhovell

…ause some of built-in functions are not in function registry. Hard code describe functions for `<>`, `!=`, `between`, and `case` for now, based on the feedback from @hvanhovell.

@yhuai

…ause some of built-in functions are not in function registry. Add usages for `<>`, `!=`, `between`, and `case`, based on the feedback from @yhuai.

…ause some of built-in functions are not in function registry. Update SQLQuerySuite to reflect changes in `describe function`.

…ause some of built-in functions are not in function registry. Take into consideration the upper and lower case of `describe function` `between` and `describe function` `case`. Fix incorrect `between` usage.

…ause some of built-in functions are not in function registry. Add tests to cover hard coded 'describe function's.

SparkQA · 2016-04-09T19:40:37Z

Test build #55436 has finished for PR 12128 at commit 4c59c81.

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

yongtang · 2016-04-09T20:11:05Z

Thanks @yhuai. The pull request has been rebased and passed the Jenkins. Please let me know if there are any other issues.

yhuai reviewed Apr 4, 2016
View reviewed changes

hvanhovell reviewed Apr 5, 2016
View reviewed changes

yongtang added 6 commits April 9, 2016 17:34

[SPARK-14335][SQL] Describe function command returns wrong output bec…

6a47284

…ause some of built-in functions are not in function registry. Hard code describe functions for `<>`, `!=`, `between`, and `case` for now, based on the feedback from @hvanhovell.

[SPARK-14335][SQL] Describe function command returns wrong output bec…

344861f

…ause some of built-in functions are not in function registry. Add usages for `<>`, `!=`, `between`, and `case`, based on the feedback from @yhuai.

[SPARK-14335][SQL] Describe function command returns wrong output bec…

9db807f

…ause some of built-in functions are not in function registry. Update SQLQuerySuite to reflect changes in `describe function`.

[SPARK-14335][SQL] Describe function command returns wrong output bec…

625c699

…ause some of built-in functions are not in function registry. Take into consideration the upper and lower case of `describe function` `between` and `describe function` `case`. Fix incorrect `between` usage.

[SPARK-14335][SQL] Describe function command returns wrong output bec…

4c59c81

…ause some of built-in functions are not in function registry. Add tests to cover hard coded 'describe function's.

asfgit closed this in cd2fed7 Apr 9, 2016

yongtang deleted the SPARK-14335 branch April 9, 2016 23:12

Conversation

yongtang commented Apr 2, 2016

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

SparkQA commented Apr 2, 2016

Uh oh!

hvanhovell commented Apr 3, 2016

Uh oh!

yhuai commented Apr 3, 2016

Uh oh!

yongtang commented Apr 3, 2016

Uh oh!

hvanhovell commented Apr 3, 2016

Uh oh!

yongtang commented Apr 3, 2016

Uh oh!

yongtang commented Apr 4, 2016

Uh oh!

yhuai commented Apr 4, 2016

Uh oh!

yhuai Apr 4, 2016

Choose a reason for hiding this comment

Uh oh!

yongtang Apr 4, 2016

Choose a reason for hiding this comment

Uh oh!

yongtang commented Apr 4, 2016

Uh oh!

SparkQA commented Apr 4, 2016

Uh oh!

SparkQA commented Apr 4, 2016

Uh oh!

SparkQA commented Apr 4, 2016

Uh oh!

SparkQA commented Apr 4, 2016

Uh oh!

yhuai Apr 4, 2016

Choose a reason for hiding this comment

Uh oh!

yongtang Apr 5, 2016

Choose a reason for hiding this comment

Uh oh!

yongtang commented Apr 5, 2016

Uh oh!

SparkQA commented Apr 5, 2016

Uh oh!

hvanhovell Apr 5, 2016

Choose a reason for hiding this comment

Uh oh!

yongtang Apr 5, 2016

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Apr 5, 2016

Uh oh!

yhuai commented Apr 9, 2016

Uh oh!

SparkQA commented Apr 9, 2016

Uh oh!

yongtang commented Apr 9, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants