[SPARK-44131][SQL] Add call_function and deprecate call_udf for Scala API #41687

beliefer · 2023-06-21T06:53:26Z

What changes were proposed in this pull request?

The Scala API exists a method call_udf used to call the user-defined functions.
In fact, call_udf also could call the builtin functions.
The behavior is confused for users.

This PR adds call_function to replace call_udf and deprecate call_udf for Scala API.

Why are the changes needed?

Fix the confusion of call_udf.

Does this PR introduce any user-facing change?

'No'.
New feature.

How was this patch tested?

Exists test cases.

beliefer · 2023-06-25T05:24:38Z

ping @cloud-fan @zhengruifeng cc @HyukjinKwon

cloud-fan · 2023-06-26T18:42:33Z

I'm fine with it as the API is the same as invoking a SQL function. cc @dongjoon-hyun @HyukjinKwon

dongjoon-hyun · 2023-06-26T18:52:45Z

sql/core/src/main/scala/org/apache/spark/sql/functions.scala

So, this new method is not udf_funcs group, right?

Yes. it's not.

dongjoon-hyun · 2023-06-26T18:54:22Z

sql/core/src/main/scala/org/apache/spark/sql/functions.scala

It seems that we need @scala.annotation.varargs.

dongjoon-hyun · 2023-06-26T18:54:48Z

sql/core/src/main/scala/org/apache/spark/sql/functions.scala

@scala.annotation.varargs?

zhengruifeng · 2023-06-26T22:16:13Z

sql/core/src/main/scala/org/apache/spark/sql/functions.scala

qq:
1, dose parameter isDistinct works for all the functions?
2, since call_function can invoke both built-in functions and udfs, shall we add a new parameter (may support options built-in-only, udf-only, global) to specify where to look up the function? so that we can invoke a built-in function even if it is overrided by udfs, e.g.

scala> import org.apache.spark.sql.functions._ import org.apache.spark.sql.functions._ scala> val df = Seq(("a", "A"), ("b", "B")).toDF("key", "value") df: org.apache.spark.sql.DataFrame = [key: string, value: string] scala> df.selectExpr("upper(key)").show +----------+ |upper(key)| +----------+ | A| | B| +----------+ scala> val foo = udf{str: String => str.toUpperCase() + "_X"} foo: org.apache.spark.sql.expressions.UserDefinedFunction = SparkUserDefinedFunction($Lambda$3089/0x000000080140f040@6ccb2162,StringType,List(Some(class[value[0]: string])),Some(class[value[0]: string]),None,true,true) scala> spark.udf.register("upper", foo.asNondeterministic()) 23/06/26 15:15:09 WARN SimpleFunctionRegistry: The function upper replaced a previously registered function. res1: org.apache.spark.sql.expressions.UserDefinedFunction = SparkUserDefinedFunction($Lambda$3089/0x000000080140f040@6ccb2162,StringType,List(Some(class[value[0]: string])),Some(class[value[0]: string]),Some(upper),true,false) scala> df.selectExpr("upper(key)").show +----------+ |upper(key)| +----------+ | A_X| | B_X| +----------+

@zhengruifeng Good point. But it seems the behavior is consistent with SQL syntax. cc @cloud-fan

For the first point, isDistinct doesn't works for all functions. isDistinct is used by aggregate or window functions.

For the second point, according to the discussion between @cloud-fan and me, it's not consistent with SQL syntax.

1, since isDistinct doesn't works for all functions, what is the behavior if user invoke unsupported functions like call_function("abs", true) ?

2, I guess we can simply make it private?

For the first point, could we add call_aggregate_function and call_window_function with isDistinct parameter ?

I think we should not expose isDistinct

OK. let's not expose isDistinct now. If we need isDistinct in future, add it then.

zhengruifeng · 2023-06-26T22:25:09Z

sql/core/src/main/scala/org/apache/spark/sql/functions.scala

I just realize that it maybe problematic in such cases, if some users happen to register a udf with the same name ceil

Good point. If we want avoid this issue, it seems we should make the built-in-only, udf-only, global as you said.

ok, if the goal of this PR is to replace call_udf with call_function, we can resolve this naming conflict issue in another PRs.

HyukjinKwon

I am fine with this too

beliefer · 2023-07-07T10:38:46Z

ping @zhengruifeng Please take a review again. cc @cloud-fan

zhengruifeng · 2023-07-10T01:48:19Z

.../src/test/scala/org/apache/spark/sql/connect/client/CheckConnectJvmClientCompatibility.scala

can we also add this new function to Spark Connect in this PR?

… API

zhengruifeng · 2023-07-10T10:11:59Z

merged to master

beliefer · 2023-07-10T11:20:18Z

@zhengruifeng @HyukjinKwon @cloud-fan @dongjoon-hyun Thank you for all!

cloud-fan · 2023-07-11T01:03:53Z

Since we are adding a new API, shall we make it more like the SQL syntax? e.g. the function name can be qualified, so that people can use it to invoke persistent functions as well.

rxin · 2023-07-12T03:46:22Z

Why would we deprecate the old one? This just creates tons of warning messages for users. If I was an end user, I'd be super annoyed that Spark just decided to rename things and generate tons of warnings with an upgrade. We don't really gain much by deprecating the existing one.

rxin · 2023-07-12T03:50:24Z

BTW it's even more "amusing" and frustrating if you look at the history.

We decided to deprecate callUDF which has existed since Spark 1.5 for call_udf in Spark 3.2, and then 3 or 4 releases later we decided to depreciate call_udf because "call_udf" can also call built-in functions, so now users need to deal with all these deprecation warnings to update their codebase to use call_function.

zhengruifeng · 2023-07-12T04:09:45Z

ok, let me remove the warning messages introduced in this PR.
For a user who just want to call a UDF (I think users are likely unware of the fact that call_udf also call built-in functions), it is meaningless to suggest he switch to a new API.

### What changes were proposed in this pull request? Revert the deprecation message changes ### Why are the changes needed? to address #41687 (comment) ### Does this PR introduce _any_ user-facing change? yes ### How was this patch tested? existing CI Closes #41950 from zhengruifeng/sql_call_function_warning. Authored-by: Ruifeng Zheng <[email protected]> Signed-off-by: Ruifeng Zheng <[email protected]>

…ion name for call_function ### What changes were proposed in this pull request? #41687 added `call_function` and deprecate `call_udf` for Scala API. Some times, the function name can be qualified, we should let users use it to invoke persistent functions as well. ### Why are the changes needed? Support qualified function name for `call_function`. ### Does this PR introduce _any_ user-facing change? 'No'. New feature. ### How was this patch tested? New test cases. Closes #41932 from beliefer/SPARK-44131_followup. Authored-by: Jiaan Geng <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>

…ion name for call_function ### What changes were proposed in this pull request? #41687 added `call_function` and deprecate `call_udf` for Scala API. Some times, the function name can be qualified, we should let users use it to invoke persistent functions as well. ### Why are the changes needed? Support qualified function name for `call_function`. ### Does this PR introduce _any_ user-facing change? 'No'. New feature. ### How was this patch tested? New test cases. Closes #41932 from beliefer/SPARK-44131_followup. Authored-by: Jiaan Geng <[email protected]> Signed-off-by: Wenchen Fan <[email protected]> (cherry picked from commit d97a4e2) Signed-off-by: Wenchen Fan <[email protected]>

github-actions bot added SQL CONNECT CORE PYTHON labels Jun 21, 2023

beliefer force-pushed the SPARK-44131 branch from c335e55 to df7c50d Compare June 25, 2023 01:41

dongjoon-hyun reviewed Jun 26, 2023

View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/functions.scala Outdated

Copy link

Member

dongjoon-hyun Jun 26, 2023 •

edited

Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@scala.annotation.varargs?

zhengruifeng reviewed Jun 26, 2023

View reviewed changes

HyukjinKwon approved these changes Jul 3, 2023

View reviewed changes

zhengruifeng reviewed Jul 10, 2023

View reviewed changes

beliefer added 10 commits July 10, 2023 11:03

[SPARK-44131][SQL] Add call_function and deprecate call_udf for Scala…

32e5a26

… API

Update code

7a185b7

Update code

64bb940

Update code

47cc5f4

Update code

3a6f044

Update code

d6e20c8

Update code

47b3121

Update cod

9ea18a4

Update code

edc4d14

Update code

c49d2cf

beliefer force-pushed the SPARK-44131 branch from 40d1722 to c49d2cf Compare July 10, 2023 04:43

Update code

0f7e98a

zhengruifeng closed this in 46dd3aa Jul 10, 2023

beliefer mentioned this pull request Jul 11, 2023

[SPARK-44131][SQL][PYTHON][CONNECT][FOLLOWUP] Support qualified function name for call_function #41932

Closed

zhengruifeng mentioned this pull request Jul 12, 2023

[SPARK-44131][SQL][FOLLOWUP] Revert the deprecation message #41950

Closed

[SPARK-44131][SQL] Add call_function and deprecate call_udf for Scala API #41687

[SPARK-44131][SQL] Add call_function and deprecate call_udf for Scala API #41687

Uh oh!

Conversation

beliefer commented Jun 21, 2023

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

beliefer commented Jun 25, 2023

Uh oh!

cloud-fan commented Jun 26, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Jun 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Jun 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhengruifeng Jun 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon left a comment

Choose a reason for hiding this comment

Uh oh!

beliefer commented Jul 7, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhengruifeng commented Jul 10, 2023

Uh oh!

beliefer commented Jul 10, 2023

Uh oh!

cloud-fan commented Jul 11, 2023

Uh oh!

rxin commented Jul 12, 2023

Uh oh!

rxin commented Jul 12, 2023

Uh oh!

zhengruifeng commented Jul 12, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

dongjoon-hyun Jun 26, 2023 •

edited

Loading

dongjoon-hyun Jun 26, 2023 •

edited

Loading

zhengruifeng Jun 26, 2023 •

edited

Loading