You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "zhengruifeng (via GitHub)" <gi...@apache.org> on 2023/09/01 14:30:02 UTC
[GitHub] [spark] zhengruifeng opened a new pull request, #42775: [SPARK-45052][SQL][PYTHON] Make functions default output column name consistent with SQL
zhengruifeng opened a new pull request, #42775:
URL: https://github.com/apache/spark/pull/42775
### What changes were proposed in this pull request?
for SQL function aliases (with `setAlias=true`) added in 3.5, using replace expression construction with `call_function` to make the output column name consistent with SQL
### Why are the changes needed?
before this PR
```
scala> val df = spark.range(0, 10)
df: org.apache.spark.sql.Dataset[Long] = [id: bigint]
scala> df.createOrReplaceTempView("t")
scala> spark.sql("SELECT TRY_SUM(id), TRY_AVG(id) FROM t")
res1: org.apache.spark.sql.DataFrame = [try_sum(id): bigint, try_avg(id): double]
scala> df.select(try_sum(col("id")), try_avg(col("id")))
res2: org.apache.spark.sql.DataFrame = [sum(id): bigint, avg(id): double]
scala>
scala> spark.sql("SELECT sign(-1), signum(-1)")
res3: org.apache.spark.sql.DataFrame = [sign(-1): double, SIGNUM(-1): double]
scala> spark.range(1).select(sign(lit(-1)), signum(lit(-1)))
res4: org.apache.spark.sql.DataFrame = [SIGNUM(-1): double, SIGNUM(-1): double]
```
after this PR
```
scala> spark.sql("SELECT TRY_SUM(id), TRY_AVG(id) FROM t")
res9: org.apache.spark.sql.DataFrame = [try_sum(id): bigint, try_avg(id): double]
scala> df.select(try_sum(col("id")), try_avg(col("id")))
res10: org.apache.spark.sql.DataFrame = [try_sum(id): bigint, try_avg(id): double]
scala>
scala> spark.sql("SELECT sign(-1), signum(-1)")
res11: org.apache.spark.sql.DataFrame = [sign(-1): double, SIGNUM(-1): double]
scala> spark.range(1).select(sign(lit(-1)), signum(lit(-1)))
res12: org.apache.spark.sql.DataFrame = [sign(-1): double, SIGNUM(-1): double]
```
### Does this PR introduce _any_ user-facing change?
yes
### How was this patch tested?
updated UT
### Was this patch authored or co-authored using generative AI tooling?
no
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #42775: [SPARK-45052][SQL][PYTHON] Make function aliases output column name consistent with SQL
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #42775:
URL: https://github.com/apache/spark/pull/42775#issuecomment-1703769617
> I'm good with this change. But from the demo, I found another issue: Do we need to maintain consistent capitalization of function names, such as: <img alt="image" width="609" src="https://user-images.githubusercontent.com/15246973/265193339-c54fb4b9-55b3-4300-8b06-ca757284e5eb.png"> 1.Whether written in uppercase or lowercase, the display is lowercase. 2.Some are lowercase and some are uppercase.
>
> **Of course, this is another issue.**
In sql, uppercase/lowercase don't affect the output column name:
```
scala> spark.sql("SELECT sign(-1), SIGN(-1), signum(-1), SIGNUM(-1)").show
+--------+--------+----------+----------+
|sign(-1)|sign(-1)|SIGNUM(-1)|SIGNUM(-1)|
+--------+--------+----------+----------+
| -1.0| -1.0| -1.0| -1.0|
+--------+--------+----------+----------+
```
It seems there is not a criterion of the style.
yeah, this PR only aims to make the newly added functions generate the same default names with SQL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on a diff in pull request #42775: [SPARK-45052][SQL][PYTHON][CONNECT] Make function aliases output column name consistent with SQL
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #42775:
URL: https://github.com/apache/spark/pull/42775#discussion_r1314347624
##########
sql/core/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -1052,15 +1049,15 @@ object functions {
* @group agg_funcs
* @since 3.5.0
*/
- def std(e: Column): Column = stddev(e)
+ def std(e: Column): Column = call_function("std", e)
/**
* Aggregate function: alias for `stddev_samp`.
*
* @group agg_funcs
* @since 1.6.0
*/
- def stddev(e: Column): Column = withAggregateFunction { StddevSamp(e.expr) }
+ def stddev(e: Column): Column = call_function("stddev", e)
Review Comment:
good catch! it was since 1.6.0.
but what about keeping this change, otherwise it make SQL/Connect(both Scala Client and Python Client) different from vanilla Spark(Python and Scala)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #42775: [SPARK-45052][SQL][PYTHON][CONNECT] Make function aliases output column name consistent with SQL
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #42775:
URL: https://github.com/apache/spark/pull/42775#discussion_r1314366796
##########
sql/core/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -1052,15 +1049,15 @@ object functions {
* @group agg_funcs
* @since 3.5.0
*/
- def std(e: Column): Column = stddev(e)
+ def std(e: Column): Column = call_function("std", e)
/**
* Aggregate function: alias for `stddev_samp`.
*
* @group agg_funcs
* @since 1.6.0
*/
- def stddev(e: Column): Column = withAggregateFunction { StddevSamp(e.expr) }
+ def stddev(e: Column): Column = call_function("stddev", e)
Review Comment:
I think we should fix the alias to `stddev` for the cases that are not.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on a diff in pull request #42775: [SPARK-45052][SQL][PYTHON][CONNECT] Make function aliases output column name consistent with SQL
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #42775:
URL: https://github.com/apache/spark/pull/42775#discussion_r1314358804
##########
sql/core/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -1052,15 +1049,15 @@ object functions {
* @group agg_funcs
* @since 3.5.0
*/
- def std(e: Column): Column = stddev(e)
+ def std(e: Column): Column = call_function("std", e)
/**
* Aggregate function: alias for `stddev_samp`.
*
* @group agg_funcs
* @since 1.6.0
*/
- def stddev(e: Column): Column = withAggregateFunction { StddevSamp(e.expr) }
+ def stddev(e: Column): Column = call_function("stddev", e)
Review Comment:
according to https://github.com/apache/spark/blob/9d28bef2f70b06cbb2f50a6814f8433fa344052e/sql/core/src/main/scala/org/apache/spark/sql/functions.scala#L54-L55, I think we can change such inconsistence to make `stddev("id")` consistent with `expr("stddev(id)"))`
```
scala> df.select(stddev("id"), expr("stddev(id)"))
res0: org.apache.spark.sql.DataFrame = [stddev_samp(id): double, stddev(id): double]
```
also cc @HyukjinKwon
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #42775: [SPARK-45052][SQL][PYTHON] Make function aliases output column name consistent with SQL
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #42775:
URL: https://github.com/apache/spark/pull/42775#issuecomment-1702873085
ping @beliefer @panbingkun @HyukjinKwon
also cc @cloud-fan
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #42775: [SPARK-45052][SQL][PYTHON] Make function aliases output column name consistent with SQL
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #42775:
URL: https://github.com/apache/spark/pull/42775#issuecomment-1703772559
another example:
before this PR:
```
scala> spark.sql("SELECT std(id), stddev(id), stddev_samp(id), stddev_pop(id) from t").show
+------------------+------------------+------------------+------------------+
| std(id)| stddev(id)| stddev_samp(id)| stddev_pop(id)|
+------------------+------------------+------------------+------------------+
|3.0276503540974917|3.0276503540974917|3.0276503540974917|2.8722813232690143|
+------------------+------------------+------------------+------------------+
scala> df.select(std(col("id")), stddev(col("id")), stddev_samp(col("id")), stddev_pop(col("id"))).show
+------------------+------------------+------------------+------------------+
| stddev_samp(id)| stddev_samp(id)| stddev_samp(id)| stddev_pop(id)|
+------------------+------------------+------------------+------------------+
|3.0276503540974917|3.0276503540974917|3.0276503540974917|2.8722813232690143|
+------------------+------------------+------------------+------------------+
```
after this PR:
```
scala> spark.sql("SELECT std(id), stddev(id), stddev_samp(id), stddev_pop(id) from t").show
+------------------+------------------+------------------+------------------+
| std(id)| stddev(id)| stddev_samp(id)| stddev_pop(id)|
+------------------+------------------+------------------+------------------+
|3.0276503540974917|3.0276503540974917|3.0276503540974917|2.8722813232690143|
+------------------+------------------+------------------+------------------+
scala> df.select(std(col("id")), stddev(col("id")), stddev_samp(col("id")), stddev_pop(col("id"))).show
+------------------+------------------+------------------+------------------+
| std(id)| stddev(id)| stddev_samp(id)| stddev_pop(id)|
+------------------+------------------+------------------+------------------+
|3.0276503540974917|3.0276503540974917|3.0276503540974917|2.8722813232690143|
+------------------+------------------+------------------+------------------+
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on a diff in pull request #42775: [SPARK-45052][SQL][PYTHON][CONNECT] Make function aliases output column name consistent with SQL
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #42775:
URL: https://github.com/apache/spark/pull/42775#discussion_r1314370284
##########
sql/core/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -1052,15 +1049,15 @@ object functions {
* @group agg_funcs
* @since 3.5.0
*/
- def std(e: Column): Column = stddev(e)
+ def std(e: Column): Column = call_function("std", e)
/**
* Aggregate function: alias for `stddev_samp`.
*
* @group agg_funcs
* @since 1.6.0
*/
- def stddev(e: Column): Column = withAggregateFunction { StddevSamp(e.expr) }
+ def stddev(e: Column): Column = call_function("stddev", e)
Review Comment:
thanks, there are still some cases introduced in older versions, I will take a look later.
For version before 3.5.0, this PR only fix `stddev` due to its relationship to the newly added `std`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #42775: [SPARK-45052][SQL][PYTHON][CONNECT] Make function aliases output column name consistent with SQL
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #42775:
URL: https://github.com/apache/spark/pull/42775#issuecomment-1704506729
merged to master, will send a separate PR for 3.5
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a diff in pull request #42775: [SPARK-45052][SQL][PYTHON][CONNECT] Make function aliases output column name consistent with SQL
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #42775:
URL: https://github.com/apache/spark/pull/42775#discussion_r1313816119
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -987,7 +987,7 @@ object functions {
* @group agg_funcs
* @since 3.5.0
*/
- def std(e: Column): Column = stddev(e)
+ def std(e: Column): Column = Column.fn("std", e)
Review Comment:
Looks good.
##########
sql/core/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -575,7 +575,7 @@ object functions {
* @group agg_funcs
* @since 3.5.0
*/
- def first_value(e: Column): Column = first(e)
+ def first_value(e: Column): Column = call_function("first_value", e)
Review Comment:
Seems good.
##########
sql/core/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -1052,15 +1049,15 @@ object functions {
* @group agg_funcs
* @since 3.5.0
*/
- def std(e: Column): Column = stddev(e)
+ def std(e: Column): Column = call_function("std", e)
/**
* Aggregate function: alias for `stddev_samp`.
*
* @group agg_funcs
* @since 1.6.0
*/
- def stddev(e: Column): Column = withAggregateFunction { StddevSamp(e.expr) }
+ def stddev(e: Column): Column = call_function("stddev", e)
Review Comment:
This one may is a break change. Even if the behavior is more suitable.
##########
python/pyspark/sql/functions.py:
##########
@@ -2385,25 +2416,54 @@ def signum(col: "ColumnOrName") -> Column:
Examples
--------
- >>> df = spark.range(1)
- >>> df.select(signum(lit(-5))).show()
- +----------+
- |SIGNUM(-5)|
- +----------+
- | -1.0|
- +----------+
-
- >>> df.select(signum(lit(6))).show()
- +---------+
- |SIGNUM(6)|
- +---------+
- | 1.0|
- +---------+
+ >>> import pyspark.sql.functions as sf
+ >>> spark.range(1).select(
+ ... sf.signum(sf.lit(-5)),
+ ... sf.signum(sf.lit(6))
+ ... ).show()
+ +----------+---------+
+ |SIGNUM(-5)|SIGNUM(6)|
+ +----------+---------+
+ | -1.0| 1.0|
+ +----------+---------+
"""
return _invoke_function_over_columns("signum", col)
-sign = signum
+@try_remote_functions
+def sign(col: "ColumnOrName") -> Column:
+ """
+ Computes the signum of the given value.
+
+ .. versionadded:: 1.4.0
+
+ .. versionchanged:: 3.4.0
Review Comment:
3.5.0 ?
##########
python/pyspark/sql/functions.py:
##########
@@ -2228,14 +2258,15 @@ def negative(col: "ColumnOrName") -> Column:
Examples
--------
- >>> spark.range(3).select(negative("id").alias("n")).show()
- +---+
- | n|
- +---+
- | 0|
- | -1|
- | -2|
- +---+
+ >>> import pyspark.sql.functions as sf
+ >>> spark.range(3).select(sf.negative("id")).show()
Review Comment:
Good change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on a diff in pull request #42775: [SPARK-45052][SQL][PYTHON][CONNECT] Make function aliases output column name consistent with SQL
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #42775:
URL: https://github.com/apache/spark/pull/42775#discussion_r1314346571
##########
python/pyspark/sql/functions.py:
##########
@@ -2385,25 +2416,54 @@ def signum(col: "ColumnOrName") -> Column:
Examples
--------
- >>> df = spark.range(1)
- >>> df.select(signum(lit(-5))).show()
- +----------+
- |SIGNUM(-5)|
- +----------+
- | -1.0|
- +----------+
-
- >>> df.select(signum(lit(6))).show()
- +---------+
- |SIGNUM(6)|
- +---------+
- | 1.0|
- +---------+
+ >>> import pyspark.sql.functions as sf
+ >>> spark.range(1).select(
+ ... sf.signum(sf.lit(-5)),
+ ... sf.signum(sf.lit(6))
+ ... ).show()
+ +----------+---------+
+ |SIGNUM(-5)|SIGNUM(6)|
+ +----------+---------+
+ | -1.0| 1.0|
+ +----------+---------+
"""
return _invoke_function_over_columns("signum", col)
-sign = signum
+@try_remote_functions
+def sign(col: "ColumnOrName") -> Column:
+ """
+ Computes the signum of the given value.
+
+ .. versionadded:: 1.4.0
+
+ .. versionchanged:: 3.4.0
Review Comment:
it should be 3.4 here, since we had `sign = signum`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on a diff in pull request #42775: [SPARK-45052][SQL][PYTHON][CONNECT] Make function aliases output column name consistent with SQL
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #42775:
URL: https://github.com/apache/spark/pull/42775#discussion_r1314347624
##########
sql/core/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -1052,15 +1049,15 @@ object functions {
* @group agg_funcs
* @since 3.5.0
*/
- def std(e: Column): Column = stddev(e)
+ def std(e: Column): Column = call_function("std", e)
/**
* Aggregate function: alias for `stddev_samp`.
*
* @group agg_funcs
* @since 1.6.0
*/
- def stddev(e: Column): Column = withAggregateFunction { StddevSamp(e.expr) }
+ def stddev(e: Column): Column = call_function("stddev", e)
Review Comment:
good catch! it was since 1.6.0.
but what about keeping this change? otherwise it make SQL/Connect(both Scala Client and Python Client) different from vanilla Spark(Python and Scala)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng closed pull request #42775: [SPARK-45052][SQL][PYTHON][CONNECT] Make function aliases output column name consistent with SQL
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng closed pull request #42775: [SPARK-45052][SQL][PYTHON][CONNECT] Make function aliases output column name consistent with SQL
URL: https://github.com/apache/spark/pull/42775
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] panbingkun commented on pull request #42775: [SPARK-45052][SQL][PYTHON] Make function aliases output column name consistent with SQL
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #42775:
URL: https://github.com/apache/spark/pull/42775#issuecomment-1703768156
I'm good with this change.
But from the demo, I found another issue:
Do we need to maintain consistent capitalization of function names, such as:
<img width="609" alt="image" src="https://github.com/apache/spark/assets/15246973/c54fb4b9-55b3-4300-8b06-ca757284e5eb">
1.Whether written in uppercase or lowercase, the display is lowercase.
2.Some are lowercase and some are uppercase.
Of course, this is another issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org