You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/11/23 12:33:00 UTC
[GitHub] [spark] zhengruifeng opened a new pull request, #38770: [SPARK-41238][CONNECT][PYTHON] Support more datatypes
zhengruifeng opened a new pull request, #38770:
URL: https://github.com/apache/spark/pull/38770
### What changes were proposed in this pull request?
1, in the sever side, make `proto_datatype` <-> `catalyst_datatype` conversion support all the built-in sql datatypes;
2, in the client side, make `proto_datatype` <-> `pyspark_catalyst_datatype` conversion support [all the datatypes that are supported in pyspark now.](https://github.com/apache/spark/blob/master/python/pyspark/sql/types.py#L60-L83)
### Why are the changes needed?
right now, only `long`, `string`, `struct` are supported
```
grpc._channel._InactiveRpcError: <_InactiveRpcError of RPC that terminated with:
status = StatusCode.UNKNOWN
details = "Does not support convert float to connect proto types."
debug_error_string = "{"created":"@1669206685.760099000","description":"Error received from peer ipv6:[::1]:15002","file":"src/core/lib/surface/call.cc","file_line":1064,"grpc_message":"Does not support convert float to connect proto types.","grpc_status":2}"
```
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
added UT
--
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 #38770: [SPARK-41238][CONNECT][PYTHON] Support more built-in datatypes
Posted by GitBox <gi...@apache.org>.
zhengruifeng closed pull request #38770: [SPARK-41238][CONNECT][PYTHON] Support more built-in datatypes
URL: https://github.com/apache/spark/pull/38770
--
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 #38770: [SPARK-41238][CONNECT][PYTHON] Support more built-in datatypes
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on PR #38770:
URL: https://github.com/apache/spark/pull/38770#issuecomment-1326934040
merged into 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
[GitHub] [spark] amaliujia commented on pull request #38770: [SPARK-41238][CONNECT][PYTHON] Support more datatypes
Posted by GitBox <gi...@apache.org>.
amaliujia commented on PR #38770:
URL: https://github.com/apache/spark/pull/38770#issuecomment-1325840723
Thanks.
Can you test both `nullable=true` and `nullable=false` case?
--
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 #38770: [SPARK-41238][CONNECT][PYTHON] Support more built-in datatypes
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on PR #38770:
URL: https://github.com/apache/spark/pull/38770#issuecomment-1326058481
> Can you test both nullable=true and nullable=false case?
the tests added in `test_schema` covers those cases
--
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 #38770: [SPARK-41238][CONNECT][PYTHON] Support more built-in datatypes
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on code in PR #38770:
URL: https://github.com/apache/spark/pull/38770#discussion_r1031939574
##########
connector/connect/src/main/protobuf/spark/connect/types.proto:
##########
@@ -26,31 +26,46 @@ option java_package = "org.apache.spark.connect.proto";
// itself but only describes it.
message DataType {
Review Comment:
maybe also need to add some UDTs(like VectorUDT) in the future, but now SQL types were completed.
--
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 #38770: [SPARK-41238][CONNECT][PYTHON] Support more built-in datatypes
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on PR #38770:
URL: https://github.com/apache/spark/pull/38770#issuecomment-1326937191
> We should also probably fix up literal support together after this (see also https://github.com/apache/spark/pull/38462#discussion_r1013933320)
will take a look
--
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] grundprinzip commented on pull request #38770: [SPARK-41238][CONNECT][PYTHON] Support more built-in datatypes
Posted by GitBox <gi...@apache.org>.
grundprinzip commented on PR #38770:
URL: https://github.com/apache/spark/pull/38770#issuecomment-1326082969
Thank you! This has caused some challenges in demoing as we didn't support many types!
--
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] grundprinzip commented on a diff in pull request #38770: [SPARK-41238][CONNECT][PYTHON] Support more built-in datatypes
Posted by GitBox <gi...@apache.org>.
grundprinzip commented on code in PR #38770:
URL: https://github.com/apache/spark/pull/38770#discussion_r1031161265
##########
connector/connect/src/main/protobuf/spark/connect/types.proto:
##########
@@ -26,31 +26,46 @@ option java_package = "org.apache.spark.connect.proto";
// itself but only describes it.
message DataType {
Review Comment:
This is a severe breaking change, we need to make sure that at some point in time our protos become stable. In particular, here the order of the members changes.
--
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] grundprinzip commented on a diff in pull request #38770: [SPARK-41238][CONNECT][PYTHON] Support more built-in datatypes
Posted by GitBox <gi...@apache.org>.
grundprinzip commented on code in PR #38770:
URL: https://github.com/apache/spark/pull/38770#discussion_r1031163802
##########
python/pyspark/sql/tests/connect/test_connect_basic.py:
##########
@@ -144,6 +144,73 @@ def test_schema(self):
schema,
)
+ # test FloatType, DoubleType, DecimalType, StringType, BooleanType, NullType
+ query = """
Review Comment:
Love the SQL test for type checking :)
--
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] amaliujia commented on pull request #38770: [SPARK-41238][CONNECT][PYTHON] Support more built-in datatypes
Posted by GitBox <gi...@apache.org>.
amaliujia commented on PR #38770:
URL: https://github.com/apache/spark/pull/38770#issuecomment-1326721579
LGTM
--
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