You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "xinrong-meng (via GitHub)" <gi...@apache.org> on 2025/05/21 20:46:11 UTC
[PR] [WIP][SPARK-52249][PS] Enable divide-by-zero with ANSI enabled [spark]
xinrong-meng opened a new pull request, #50972:
URL: https://github.com/apache/spark/pull/50972
### What changes were proposed in this pull request?
Enable divide-by-zero with ANSI enabled
### Why are the changes needed?
Part of https://issues.apache.org/jira/browse/SPARK-52169
### Does this PR introduce _any_ user-facing change?
Yes, divide-by-zero is enabled with ANSI enabled
### How was this patch tested?
Unit tests
### 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-52249][PS] Enable divide-by-zero for truediv with ANSI enabled [spark]
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on code in PR #50972:
URL: https://github.com/apache/spark/pull/50972#discussion_r2112864716
##########
python/pyspark/pandas/utils.py:
##########
@@ -1070,6 +1070,14 @@ def xor(df1: PySparkDataFrame, df2: PySparkDataFrame) -> PySparkDataFrame:
)
+def is_ansi_mode_enabled() -> bool:
Review Comment:
Shall we explicitly pass `spark`? `SparkSession.getActiveSession()` is not light.
--
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-52249][PS] Enable divide-by-zero with ANSI enabled [spark]
Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng commented on code in PR #50972:
URL: https://github.com/apache/spark/pull/50972#discussion_r2112495338
##########
python/pyspark/pandas/data_type_ops/num_ops.py:
##########
@@ -247,14 +247,22 @@ def truediv(self, left: IndexOpsLike, right: Any) -> SeriesOrIndex:
_sanitize_list_like(right)
if not is_valid_operand_for_numeric_arithmetic(right):
raise TypeError("True division can not be applied to given types.")
+ right = transform_boolean_operand_to_numeric(right, spark_type=left.spark.data_type)
def truediv(left: PySparkColumn, right: Any) -> PySparkColumn:
- return F.when(
- F.lit(right != 0) | F.lit(right).isNull(),
- left.__div__(right),
- ).otherwise(F.lit(np.inf).__div__(left))
+ if not get_option("compute.ansi_mode_support"):
Review Comment:
Sorry would you mind clarifying what you meant?
--
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-52249][PS] Enable divide-by-zero for truediv with ANSI enabled [spark]
Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng commented on code in PR #50972:
URL: https://github.com/apache/spark/pull/50972#discussion_r2114490959
##########
python/pyspark/pandas/tests/computation/test_binary_ops.py:
##########
@@ -111,7 +111,6 @@ def test_binary_operator_sub(self):
psdf = ps.DataFrame({"a": ["x"], "b": ["y"]})
self.assertRaisesRegex(TypeError, ks_err_msg, lambda: psdf["a"] - psdf["b"])
- @unittest.skipIf(is_ansi_mode_test, ansi_mode_not_supported_message)
Review Comment:
Can I follow up with https://issues.apache.org/jira/browse/SPARK-52349 on that if you don't mind, in order to unblock the other pr?
--
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-52249][PS] Enable divide-by-zero for truediv with ANSI enabled [spark]
Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng commented on code in PR #50972:
URL: https://github.com/apache/spark/pull/50972#discussion_r2114766651
##########
python/pyspark/pandas/utils.py:
##########
@@ -1070,6 +1070,14 @@ def xor(df1: PySparkDataFrame, df2: PySparkDataFrame) -> PySparkDataFrame:
)
+def is_ansi_mode_enabled() -> bool:
Review Comment:
Adjusted.
--
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-52249][PS] Enable divide-by-zero for truediv with ANSI enabled [spark]
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on code in PR #50972:
URL: https://github.com/apache/spark/pull/50972#discussion_r2114503034
##########
python/pyspark/pandas/tests/computation/test_binary_ops.py:
##########
@@ -111,7 +111,6 @@ def test_binary_operator_sub(self):
psdf = ps.DataFrame({"a": ["x"], "b": ["y"]})
self.assertRaisesRegex(TypeError, ks_err_msg, lambda: psdf["a"] - psdf["b"])
- @unittest.skipIf(is_ansi_mode_test, ansi_mode_not_supported_message)
Review Comment:
Sure, that's fine. 👍
--
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-52249][PS] Enable divide-by-zero for numeric truediv with ANSI enabled [spark]
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on PR #50972:
URL: https://github.com/apache/spark/pull/50972#issuecomment-2923593292
Thanks! merging to master.
--
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-52249][PS] Enable divide-by-zero with ANSI enabled [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #50972:
URL: https://github.com/apache/spark/pull/50972#discussion_r2110687998
##########
python/pyspark/pandas/data_type_ops/num_ops.py:
##########
@@ -247,14 +247,22 @@ def truediv(self, left: IndexOpsLike, right: Any) -> SeriesOrIndex:
_sanitize_list_like(right)
if not is_valid_operand_for_numeric_arithmetic(right):
raise TypeError("True division can not be applied to given types.")
+ right = transform_boolean_operand_to_numeric(right, spark_type=left.spark.data_type)
def truediv(left: PySparkColumn, right: Any) -> PySparkColumn:
- return F.when(
- F.lit(right != 0) | F.lit(right).isNull(),
- left.__div__(right),
- ).otherwise(F.lit(np.inf).__div__(left))
+ if not get_option("compute.ansi_mode_support"):
Review Comment:
can we optimize out this `get_option` which needs a separate Config RPC?
I guess we can just use the new branch
--
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-52249][PS] Enable divide-by-zero for truediv with ANSI enabled [spark]
Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng commented on code in PR #50972:
URL: https://github.com/apache/spark/pull/50972#discussion_r2112826897
##########
python/pyspark/pandas/data_type_ops/num_ops.py:
##########
@@ -247,14 +247,22 @@ def truediv(self, left: IndexOpsLike, right: Any) -> SeriesOrIndex:
_sanitize_list_like(right)
if not is_valid_operand_for_numeric_arithmetic(right):
raise TypeError("True division can not be applied to given types.")
+ right = transform_boolean_operand_to_numeric(right, spark_type=left.spark.data_type)
def truediv(left: PySparkColumn, right: Any) -> PySparkColumn:
- return F.when(
- F.lit(right != 0) | F.lit(right).isNull(),
- left.__div__(right),
- ).otherwise(F.lit(np.inf).__div__(left))
+ if not get_option("compute.ansi_mode_support"):
Review Comment:
Makes sense! Extracted the config check "is_ansi_mode_enabled" and adjusted accordingly.
--
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-52249][PS] Enable divide-by-zero with ANSI enabled [spark]
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on code in PR #50972:
URL: https://github.com/apache/spark/pull/50972#discussion_r2112725869
##########
python/pyspark/pandas/data_type_ops/num_ops.py:
##########
@@ -247,14 +247,22 @@ def truediv(self, left: IndexOpsLike, right: Any) -> SeriesOrIndex:
_sanitize_list_like(right)
if not is_valid_operand_for_numeric_arithmetic(right):
raise TypeError("True division can not be applied to given types.")
+ right = transform_boolean_operand_to_numeric(right, spark_type=left.spark.data_type)
def truediv(left: PySparkColumn, right: Any) -> PySparkColumn:
- return F.when(
- F.lit(right != 0) | F.lit(right).isNull(),
- left.__div__(right),
- ).otherwise(F.lit(np.inf).__div__(left))
+ if not get_option("compute.ansi_mode_support"):
Review Comment:
Shall we check both `spark.sql.ansi.enabled` and it to avoid any changes for non-ANSI?
Also related, could you extract the config check to some util method to reuse it in other places.
--
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-52249][PS] Enable divide-by-zero with ANSI enabled [spark]
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on code in PR #50972:
URL: https://github.com/apache/spark/pull/50972#discussion_r2112727676
##########
python/pyspark/pandas/tests/computation/test_binary_ops.py:
##########
@@ -111,7 +111,6 @@ def test_binary_operator_sub(self):
psdf = ps.DataFrame({"a": ["x"], "b": ["y"]})
self.assertRaisesRegex(TypeError, ks_err_msg, lambda: psdf["a"] - psdf["b"])
- @unittest.skipIf(is_ansi_mode_test, ansi_mode_not_supported_message)
Review Comment:
Could you also remove skipping at other places caused by the division error?
--
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-52249][PS] Enable divide-by-zero with ANSI enabled [spark]
Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng commented on PR #50972:
URL: https://github.com/apache/spark/pull/50972#issuecomment-2905722224
Should be merged after https://github.com/apache/spark/pull/50988
--
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-52249][PS] Enable divide-by-zero for numeric truediv with ANSI enabled [spark]
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin closed pull request #50972: [SPARK-52249][PS] Enable divide-by-zero for numeric truediv with ANSI enabled
URL: https://github.com/apache/spark/pull/50972
--
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-52249][PS] Enable divide-by-zero with ANSI enabled [spark]
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on code in PR #50972:
URL: https://github.com/apache/spark/pull/50972#discussion_r2112725869
##########
python/pyspark/pandas/data_type_ops/num_ops.py:
##########
@@ -247,14 +247,22 @@ def truediv(self, left: IndexOpsLike, right: Any) -> SeriesOrIndex:
_sanitize_list_like(right)
if not is_valid_operand_for_numeric_arithmetic(right):
raise TypeError("True division can not be applied to given types.")
+ right = transform_boolean_operand_to_numeric(right, spark_type=left.spark.data_type)
def truediv(left: PySparkColumn, right: Any) -> PySparkColumn:
- return F.when(
- F.lit(right != 0) | F.lit(right).isNull(),
- left.__div__(right),
- ).otherwise(F.lit(np.inf).__div__(left))
+ if not get_option("compute.ansi_mode_support"):
Review Comment:
Shall we check both `spark.sql.ansi.enabled` and it to avoid any changes for non-ANSI? i.e., change the code only when ANSI is enabled and `compute.ansi_mode_support` is `True`.
Also related, could you extract the config check to some util method to reuse it in other places.
--
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-52249][PS] Enable divide-by-zero with ANSI enabled [spark]
Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on code in PR #50972:
URL: https://github.com/apache/spark/pull/50972#discussion_r2112727676
##########
python/pyspark/pandas/tests/computation/test_binary_ops.py:
##########
@@ -111,7 +111,6 @@ def test_binary_operator_sub(self):
psdf = ps.DataFrame({"a": ["x"], "b": ["y"]})
self.assertRaisesRegex(TypeError, ks_err_msg, lambda: psdf["a"] - psdf["b"])
- @unittest.skipIf(is_ansi_mode_test, ansi_mode_not_supported_message)
Review Comment:
Could you also try to find where we can remove skipping caused by the division error?
--
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