You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "panbingkun (via GitHub)" <gi...@apache.org> on 2023/08/16 11:07:49 UTC
[GitHub] [spark] panbingkun opened a new pull request, #42513: [SPARK-44827][PYTHON][TESTS] Fix test_assert_approx_equal_decimaltype_custom_rtol_pass when SPARK_ANSI_SQL_MODE=true
panbingkun opened a new pull request, #42513:
URL: https://github.com/apache/spark/pull/42513
### What changes were proposed in this pull request?
The pr aims to fix `test_assert_approx_equal_decimaltype_custom_rtol_pass` when SPARK_ANSI_SQL_MODE=true
### Why are the changes needed?
Make pyspark test happy.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
- Pass GA.
- Manually test:
--
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 #42513: [SPARK-44827][PYTHON][TESTS] Fix test when ansi mode enabled
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #42513:
URL: https://github.com/apache/spark/pull/42513#discussion_r1302846694
##########
python/pyspark/sql/dataframe.py:
##########
@@ -3908,8 +3908,21 @@ def union(self, other: "DataFrame") -> "DataFrame":
Example 2: Combining two DataFrames with different schemas
>>> from pyspark.sql.functions import lit
- >>> df1 = spark.createDataFrame([("Alice", 1), ("Bob", 2)], ["name", "id"])
- >>> df2 = spark.createDataFrame([(3, "Charlie"), (4, "Dave")], ["id", "name"])
+ >>> from pyspark.sql.types import StructType, StructField, StringType
+ >>> schema1 = StructType(
Review Comment:
Can you also use a different type? e.g., `LongType` vs `IntegerType`.
Also, you can specify the type via DDL string format that I think is simpler.
e.g., `schema="v1 LONG, v2 INT"`
--
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 #42513: [SPARK-44827][PYTHON][TESTS] Fix test when ansi mode enabled
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #42513:
URL: https://github.com/apache/spark/pull/42513#discussion_r1302314216
##########
python/pyspark/testing/utils.py:
##########
@@ -411,8 +412,8 @@ def assertDataFrameEqual(
Note that schema equality is checked only when `expected` is a DataFrame (not a list of Rows).
- For DataFrames with float values, assertDataFrame asserts approximate equality.
- Two float values a and b are approximately equal if the following equation is True:
+ For DataFrames with float/Decimal values, assertDataFrame asserts approximate equality.
Review Comment:
```suggestion
For DataFrames with float/decimal values, assertDataFrame asserts approximate equality.
```
##########
python/pyspark/testing/utils.py:
##########
@@ -411,8 +412,8 @@ def assertDataFrameEqual(
Note that schema equality is checked only when `expected` is a DataFrame (not a list of Rows).
- For DataFrames with float values, assertDataFrame asserts approximate equality.
- Two float values a and b are approximately equal if the following equation is True:
+ For DataFrames with float/Decimal values, assertDataFrame asserts approximate equality.
+ Two float/Decimal values a and b are approximately equal if the following equation is True:
Review Comment:
```suggestion
Two float/decimal values a and b are approximately equal if the following equation is True:
```
--
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 #42513: [SPARK-44827][PYTHON][TESTS] Fix test when ansi mode enabled
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #42513:
URL: https://github.com/apache/spark/pull/42513#issuecomment-1689111917
All 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
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #42513: [SPARK-44827][PYTHON][TESTS] Fix test when ansi mode enabled
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #42513:
URL: https://github.com/apache/spark/pull/42513#discussion_r1302314087
##########
python/pyspark/sql/functions.py:
##########
@@ -7802,8 +7802,20 @@ def to_unix_timestamp(
>>> spark.conf.set("spark.sql.session.timeZone", "America/Los_Angeles")
>>> df = spark.createDataFrame([("2016-04-08",)], ["e"])
- >>> df.select(to_unix_timestamp(df.e).alias('r')).collect()
Review Comment:
I think we can remove this whole example for now. This is a user-facing documentation that shows an example so we won't necessarily show the failure example
--
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 #42513: [SPARK-44827][PYTHON][TESTS] Fix test_assert_approx_equal_decimaltype_custom_rtol_pass when SPARK_ANSI_SQL_MODE=true
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #42513:
URL: https://github.com/apache/spark/pull/42513#issuecomment-1680404690
<img width="1417" alt="image" src="https://github.com/apache/spark/assets/15246973/0d087762-2e7b-4cb0-b1cc-0c2a7f1635ce">
--
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 #42513: [SPARK-44827][PYTHON][TESTS] Fix test when ansi mode enabled
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #42513:
URL: https://github.com/apache/spark/pull/42513#discussion_r1302314334
##########
python/pyspark/testing/utils.py:
##########
@@ -539,6 +540,9 @@ def compare_vals(val1, val2):
elif isinstance(val1, float) and isinstance(val2, float):
if abs(val1 - val2) > (atol + rtol * abs(val2)):
return False
+ elif isinstance(val1, Decimal) and isinstance(val2, Decimal):
Review Comment:
@asl3 FYI
--
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 #42513: [SPARK-44827][PYTHON][TESTS] Fix test when ansi mode enabled
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #42513:
URL: https://github.com/apache/spark/pull/42513#issuecomment-1681535866
cc @asl3
--
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] LuciferYang commented on pull request #42513: [SPARK-44827][PYTHON][TESTS] Fix test_assert_approx_equal_decimaltype_custom_rtol_pass when SPARK_ANSI_SQL_MODE=true
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #42513:
URL: https://github.com/apache/spark/pull/42513#issuecomment-1680409386
cc @HyukjinKwon @ueshin @zhengruifeng FYI
--
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 #42513: [SPARK-44827][PYTHON][TESTS] Fix test when ansi mode enabled
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #42513:
URL: https://github.com/apache/spark/pull/42513#discussion_r1302313324
##########
python/pyspark/sql/dataframe.py:
##########
@@ -3912,16 +3912,29 @@ def union(self, other: "DataFrame") -> "DataFrame":
>>> df2 = spark.createDataFrame([(3, "Charlie"), (4, "Dave")], ["id", "name"])
>>> df1 = df1.withColumn("age", lit(30))
>>> df2 = df2.withColumn("age", lit(40))
- >>> df3 = df1.union(df2)
- >>> df3.show()
- +-----+-------+---+
- | name| id|age|
- +-----+-------+---+
- |Alice| 1| 30|
- | Bob| 2| 30|
- | 3|Charlie| 40|
- | 4| Dave| 40|
- +-----+-------+---+
+ >>> if spark.conf.get("spark.sql.ansi.enabled") == "false":
Review Comment:
Can we fix the input types, for example, floats and ints so it can pass whether `spark.sql.ansi.enabled` is on or off?
--
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 #42513: [SPARK-44827][PYTHON][TESTS] Fix test when ansi mode enabled
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #42513:
URL: https://github.com/apache/spark/pull/42513#issuecomment-1689726240
Before merging, I need to change the parameters modified for testing back.
--
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 closed pull request #42513: [SPARK-44827][PYTHON][TESTS] Fix test when ansi mode enabled
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon closed pull request #42513: [SPARK-44827][PYTHON][TESTS] Fix test when ansi mode enabled
URL: https://github.com/apache/spark/pull/42513
--
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 a diff in pull request #42513: [SPARK-44827][PYTHON][TESTS] Fix `test_assert_approx_equal_decimaltype_custom_rtol_pass` when ansi mode enabled
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #42513:
URL: https://github.com/apache/spark/pull/42513#discussion_r1295760142
##########
.github/workflows/build_and_test.yml:
##########
@@ -379,6 +379,7 @@ jobs:
SKIP_PACKAGING: true
METASPACE_SIZE: 1g
BRANCH: ${{ inputs.branch }}
+ SPARK_ANSI_SQL_MODE: true
Review Comment:
Only for test
--
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 #42513: [SPARK-44827][PYTHON][TESTS] Fix test when ansi mode enabled
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #42513:
URL: https://github.com/apache/spark/pull/42513#discussion_r1299463703
##########
python/pyspark/sql/dataframe.py:
##########
@@ -3793,6 +3793,8 @@ def union(self, other: "DataFrame") -> "DataFrame":
Example 2: Combining two DataFrames with different schemas
>>> from pyspark.sql.functions import lit
+ >>> old_value = spark.conf.get("spark.sql.ansi.enabled")
+ >>> spark.conf.set("spark.sql.ansi.enabled", "false")
Review Comment:
can we fix the example instead of disabling?
--
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 pull request #42513: [SPARK-44827][PYTHON][TESTS] Fix test when ansi mode enabled
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #42513:
URL: https://github.com/apache/spark/pull/42513#issuecomment-1692571131
Merged 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