You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "dtenedor (via GitHub)" <gi...@apache.org> on 2024/08/23 00:18:35 UTC
[PR] [SPARK-49306][Python] Create SQL function aliases for 'zeroifnull' and 'nullifzero' [spark]
dtenedor opened a new pull request, #47851:
URL: https://github.com/apache/spark/pull/47851
### What changes were proposed in this pull request?
In https://github.com/apache/spark/pull/47817 we added new SQL functions `zeroifnull` and `nullifzero`.
In this PR we add Scala and Python DataFrame API endpoints for them.
For example, in Scala:
```
test("nullifzero function") {
val df = Seq((0)).toDF("a")
checkAnswer(df.selectExpr("nullifzero(0)"), Seq(Row(null)))
checkAnswer(df.select(nullifzero(lit(0))), Seq(Row(null)))
checkAnswer(df.selectExpr("nullifzero(a)"), Seq(Row(null)))
checkAnswer(df.select(nullifzero(lit(5))), Seq(Row(5)))
}
test("zeroifnull function") {
val df = Seq[(Integer)]((null)).toDF("a")
checkAnswer(df.selectExpr("zeroifnull(null)"), Seq(Row(0)))
checkAnswer(df.select(nullifzero(lit(null))), Seq(Row(0)))
checkAnswer(df.selectExpr("zeroifnull(a)"), Seq(Row(0)))
checkAnswer(df.select(zeroifnull(lit(5))), Seq(Row(5)))
}
```
### Why are the changes needed?
This improves DataFrame parity with the SQL API.
### Does this PR introduce _any_ user-facing change?
Yes, see above.
### How was this patch tested?
This PR adds unit test coverage.
### 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
Re: [PR] [SPARK-49306][PYTHON][SQL] Create SQL function aliases for 'zeroifnull' and 'nullifzero' [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #47851:
URL: https://github.com/apache/spark/pull/47851#issuecomment-2316033002
+1, LGTM. Merging to mater.
Thank you, @dtenedor and @HyukjinKwon for review.
--
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
Re: [PR] [SPARK-49306][Python] Create SQL function aliases for 'zeroifnull' and 'nullifzero' [spark]
Posted by "dtenedor (via GitHub)" <gi...@apache.org>.
dtenedor commented on PR #47851:
URL: https://github.com/apache/spark/pull/47851#issuecomment-2305957480
cc @HyukjinKwon @MaxGekk
--
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
Re: [PR] [SPARK-49306][PYTHON][SQL] Create SQL function aliases for 'zeroifnull' and 'nullifzero' [spark]
Posted by "dtenedor (via GitHub)" <gi...@apache.org>.
dtenedor commented on code in PR #47851:
URL: https://github.com/apache/spark/pull/47851#discussion_r1731907437
##########
sql/core/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -7866,6 +7874,14 @@ object functions {
*/
def nvl2(col1: Column, col2: Column, col3: Column): Column = Column.fn("nvl2", col1, col2, col3)
+ /**
+ * Returns zero if `col` is null, or `col` otherwise.
+ *
+ * @group conditional_funcs
+ * @since 4.0.0
+ */
+ def zeroifnull(col: Column): Column = Column.fn("zeroifnull", col)
Review Comment:
Sure, this is done.
##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -20681,6 +20681,26 @@ def nullif(col1: "ColumnOrName", col2: "ColumnOrName") -> Column:
return _invoke_function_over_columns("nullif", col1, col2)
+@_try_remote_functions
+def nullifzero(col: "ColumnOrName") -> Column:
+ """
+ Returns null if `col` is equal to zero, or `col` otherwise.
+
+ .. versionadded:: 4.0.0
+
+ Parameters
+ ----------
+ col : :class:`~pyspark.sql.Column` or str
+
+ Examples
+ --------
+ >>> df = spark.createDataFrame([(0,), (1,)], ["a"])
+ >>> df.select(nullifzero(df.a).alias('r')).collect()
+ [Row(r=None), Row(r=1)]
Review Comment:
Sure, this is done.
##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -20724,6 +20744,26 @@ def nvl2(col1: "ColumnOrName", col2: "ColumnOrName", col3: "ColumnOrName") -> Co
return _invoke_function_over_columns("nvl2", col1, col2, col3)
+@_try_remote_functions
+def zeroifnull(col: "ColumnOrName") -> Column:
+ """
+ Returns zero if `col` is null, or `col` otherwise.
+
+ .. versionadded:: 4.0.0
+
+ Parameters
+ ----------
+ col : :class:`~pyspark.sql.Column` or str
+
+ Examples
+ --------
+ >>> df = spark.createDataFrame([(None,), (1,)], ["a"])
+ >>> df.select(zeroifnull(df.a).alias('r')).collect()
Review Comment:
sounds good, updated!
##########
sql/core/src/test/scala/org/apache/spark/sql/DataFrameFunctionsSuite.scala:
##########
@@ -331,6 +331,15 @@ class DataFrameFunctionsSuite extends QueryTest with SharedSparkSession {
checkAnswer(df.select(nullif(lit(5), lit(5))), Seq(Row(null)))
}
+ test("nullifzero function") {
+ val df = Seq((0)).toDF("a")
+ checkAnswer(df.selectExpr("nullifzero(0)"), Seq(Row(null)))
+ checkAnswer(df.select(nullifzero(lit(0))), Seq(Row(null)))
+
+ checkAnswer(df.selectExpr("nullifzero(a)"), Seq(Row(null)))
Review Comment:
sounds good, this is done.
##########
sql/core/src/test/scala/org/apache/spark/sql/DataFrameFunctionsSuite.scala:
##########
@@ -331,6 +331,15 @@ class DataFrameFunctionsSuite extends QueryTest with SharedSparkSession {
checkAnswer(df.select(nullif(lit(5), lit(5))), Seq(Row(null)))
}
+ test("nullifzero function") {
+ val df = Seq((0)).toDF("a")
+ checkAnswer(df.selectExpr("nullifzero(0)"), Seq(Row(null)))
Review Comment:
sounds good, removed.
--
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
Re: [PR] [SPARK-49306][Python] Create SQL function aliases for 'zeroifnull' and 'nullifzero' [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #47851:
URL: https://github.com/apache/spark/pull/47851#discussion_r1728076186
##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -20681,6 +20681,26 @@ def nullif(col1: "ColumnOrName", col2: "ColumnOrName") -> Column:
return _invoke_function_over_columns("nullif", col1, col2)
+@_try_remote_functions
+def nullifzero(col: "ColumnOrName") -> Column:
+ """
+ Returns null if `col` is equal to zero, or `col` otherwise.
+
+ .. versionadded:: 4.0.0
+
+ Parameters
+ ----------
+ col : :class:`~pyspark.sql.Column` or str
+
+ Examples
+ --------
+ >>> df = spark.createDataFrame([(0,), (1,)], ["a"])
+ >>> df.select(nullifzero(df.a).alias('r')).collect()
+ [Row(r=None), Row(r=1)]
Review Comment:
Could we add a test in the actual `test_functions.py`? Then the test will be automatically shared between Spark Connect and Spark Classic
--
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
Re: [PR] [SPARK-49306][PYTHON][SQL] Create SQL function aliases for 'zeroifnull' and 'nullifzero' [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #47851:
URL: https://github.com/apache/spark/pull/47851#discussion_r1732363538
##########
sql/core/src/test/scala/org/apache/spark/sql/DataFrameFunctionsSuite.scala:
##########
@@ -331,6 +331,54 @@ class DataFrameFunctionsSuite extends QueryTest with SharedSparkSession {
checkAnswer(df.select(nullif(lit(5), lit(5))), Seq(Row(null)))
}
+ test("nullifzero function") {
+ withTable("t") {
+ // Here we exercise a non-nullable, non-foldable column.
+ sql("create table t(col int not null) using csv")
+ sql("insert into t values (0)")
+ val df = sql("select col from t")
+ checkAnswer(df.selectExpr("nullifzero(col)"), Seq(Row(null)))
Review Comment:
@dtenedor I do understand that the functions from Scala API call SQL functions, but could you test directly the added API.
--
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
Re: [PR] [SPARK-49306][PYTHON][SQL] Create SQL function aliases for 'zeroifnull' and 'nullifzero' [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #47851:
URL: https://github.com/apache/spark/pull/47851#discussion_r1734290542
##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -20681,6 +20681,31 @@ def nullif(col1: "ColumnOrName", col2: "ColumnOrName") -> Column:
return _invoke_function_over_columns("nullif", col1, col2)
+@_try_remote_functions
+def nullifzero(col: "ColumnOrName") -> Column:
+ """
+ Returns null if `col` is equal to zero, or `col` otherwise.
+
+ .. versionadded:: 4.0.0
+
+ Parameters
+ ----------
+ col : :class:`~pyspark.sql.Column` or str
+
+ Examples
+ --------
+ >>> df = spark.createDataFrame([(0,), (1,)], ["a"])
+ >>> df.select(nullifzero(df.a).alias("result")).show()
+ +------+
+ |result|
+ +------+
+ | None|
Review Comment:
Why do you expect `None` if the function `nullifzero()` should return `NULL` for `0`.
--
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
Re: [PR] [SPARK-49306][Python] Create SQL function aliases for 'zeroifnull' and 'nullifzero' [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #47851:
URL: https://github.com/apache/spark/pull/47851#discussion_r1728076673
##########
sql/core/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -7866,6 +7874,14 @@ object functions {
*/
def nvl2(col1: Column, col2: Column, col3: Column): Column = Column.fn("nvl2", col1, col2, col3)
+ /**
+ * Returns zero if `col` is null, or `col` otherwise.
+ *
+ * @group conditional_funcs
+ * @since 4.0.0
+ */
+ def zeroifnull(col: Column): Column = Column.fn("zeroifnull", col)
Review Comment:
Would need to implement this for Spark Connect in Scala too (e.g., `connector/connect/client/.../functions.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
Re: [PR] [SPARK-49306][PYTHON][SQL] Create SQL function aliases for 'zeroifnull' and 'nullifzero' [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk closed pull request #47851: [SPARK-49306][PYTHON][SQL] Create SQL function aliases for 'zeroifnull' and 'nullifzero'
URL: https://github.com/apache/spark/pull/47851
--
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
Re: [PR] [SPARK-49306][PYTHON][SQL] Create SQL function aliases for 'zeroifnull' and 'nullifzero' [spark]
Posted by "dtenedor (via GitHub)" <gi...@apache.org>.
dtenedor commented on code in PR #47851:
URL: https://github.com/apache/spark/pull/47851#discussion_r1733768552
##########
sql/core/src/test/scala/org/apache/spark/sql/DataFrameFunctionsSuite.scala:
##########
@@ -331,6 +331,54 @@ class DataFrameFunctionsSuite extends QueryTest with SharedSparkSession {
checkAnswer(df.select(nullif(lit(5), lit(5))), Seq(Row(null)))
}
+ test("nullifzero function") {
+ withTable("t") {
+ // Here we exercise a non-nullable, non-foldable column.
+ sql("create table t(col int not null) using csv")
+ sql("insert into t values (0)")
+ val df = sql("select col from t")
+ checkAnswer(df.selectExpr("nullifzero(col)"), Seq(Row(null)))
Review Comment:
Sounds good, done.
--
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
Re: [PR] [SPARK-49306][PYTHON][SQL] Create SQL function aliases for 'zeroifnull' and 'nullifzero' [spark]
Posted by "dtenedor (via GitHub)" <gi...@apache.org>.
dtenedor commented on code in PR #47851:
URL: https://github.com/apache/spark/pull/47851#discussion_r1734989936
##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -20681,6 +20681,31 @@ def nullif(col1: "ColumnOrName", col2: "ColumnOrName") -> Column:
return _invoke_function_over_columns("nullif", col1, col2)
+@_try_remote_functions
+def nullifzero(col: "ColumnOrName") -> Column:
+ """
+ Returns null if `col` is equal to zero, or `col` otherwise.
+
+ .. versionadded:: 4.0.0
+
+ Parameters
+ ----------
+ col : :class:`~pyspark.sql.Column` or str
+
+ Examples
+ --------
+ >>> df = spark.createDataFrame([(0,), (1,)], ["a"])
+ >>> df.select(nullifzero(df.a).alias("result")).show()
+ +------+
+ |result|
+ +------+
+ | None|
Review Comment:
Fixed this; it was indeed a typo and should say `NULL` instead of `None`.
--
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
Re: [PR] [SPARK-49306][PYTHON][SQL] Create SQL function aliases for 'zeroifnull' and 'nullifzero' [spark]
Posted by "dtenedor (via GitHub)" <gi...@apache.org>.
dtenedor commented on code in PR #47851:
URL: https://github.com/apache/spark/pull/47851#discussion_r1731907437
##########
sql/core/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -7866,6 +7874,14 @@ object functions {
*/
def nvl2(col1: Column, col2: Column, col3: Column): Column = Column.fn("nvl2", col1, col2, col3)
+ /**
+ * Returns zero if `col` is null, or `col` otherwise.
+ *
+ * @group conditional_funcs
+ * @since 4.0.0
+ */
+ def zeroifnull(col: Column): Column = Column.fn("zeroifnull", col)
Review Comment:
Sure, this is done.
(edit: this file seems to be gone after syncing)
##########
sql/core/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -7866,6 +7874,14 @@ object functions {
*/
def nvl2(col1: Column, col2: Column, col3: Column): Column = Column.fn("nvl2", col1, col2, col3)
+ /**
+ * Returns zero if `col` is null, or `col` otherwise.
+ *
+ * @group conditional_funcs
+ * @since 4.0.0
+ */
+ def zeroifnull(col: Column): Column = Column.fn("zeroifnull", col)
Review Comment:
Sure, this is done.
(edit: this file now seems to be gone after syncing)
--
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
Re: [PR] [SPARK-49306][PYTHON][SQL] Create SQL function aliases for 'zeroifnull' and 'nullifzero' [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #47851:
URL: https://github.com/apache/spark/pull/47851#discussion_r1728463327
##########
sql/core/src/test/scala/org/apache/spark/sql/DataFrameFunctionsSuite.scala:
##########
@@ -331,6 +331,15 @@ class DataFrameFunctionsSuite extends QueryTest with SharedSparkSession {
checkAnswer(df.select(nullif(lit(5), lit(5))), Seq(Row(null)))
}
+ test("nullifzero function") {
+ val df = Seq((0)).toDF("a")
+ checkAnswer(df.selectExpr("nullifzero(0)"), Seq(Row(null)))
Review Comment:
You have already tested the SQL function in the previous PR. I do believe this check is not necessary.
##########
sql/core/src/test/scala/org/apache/spark/sql/DataFrameFunctionsSuite.scala:
##########
@@ -331,6 +331,15 @@ class DataFrameFunctionsSuite extends QueryTest with SharedSparkSession {
checkAnswer(df.select(nullif(lit(5), lit(5))), Seq(Row(null)))
}
+ test("nullifzero function") {
+ val df = Seq((0)).toDF("a")
+ checkAnswer(df.selectExpr("nullifzero(0)"), Seq(Row(null)))
+ checkAnswer(df.select(nullifzero(lit(0))), Seq(Row(null)))
+
+ checkAnswer(df.selectExpr("nullifzero(a)"), Seq(Row(null)))
Review Comment:
Let's skip this and add more tests for:
1. Different types: maps, arrays, dates and so on
2. Not-nullable column
3. Types support ordering and do not support
4. Non-foldable column
--
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
Re: [PR] [SPARK-49306][PYTHON][SQL] Create SQL function aliases for 'zeroifnull' and 'nullifzero' [spark]
Posted by "allisonwang-db (via GitHub)" <gi...@apache.org>.
allisonwang-db commented on code in PR #47851:
URL: https://github.com/apache/spark/pull/47851#discussion_r1729461256
##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -20724,6 +20744,26 @@ def nvl2(col1: "ColumnOrName", col2: "ColumnOrName", col3: "ColumnOrName") -> Co
return _invoke_function_over_columns("nvl2", col1, col2, col3)
+@_try_remote_functions
+def zeroifnull(col: "ColumnOrName") -> Column:
+ """
+ Returns zero if `col` is null, or `col` otherwise.
+
+ .. versionadded:: 4.0.0
+
+ Parameters
+ ----------
+ col : :class:`~pyspark.sql.Column` or str
+
+ Examples
+ --------
+ >>> df = spark.createDataFrame([(None,), (1,)], ["a"])
+ >>> df.select(zeroifnull(df.a).alias('r')).collect()
Review Comment:
super nit: `df.show()` is more user-friendly :)
--
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