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/02/01 04:47:10 UTC
[GitHub] [spark] zhengruifeng opened a new pull request, #39835: [SPARK-42268][CONNECT][PYTHON] Add UserDefinedType in protos
zhengruifeng opened a new pull request, #39835:
URL: https://github.com/apache/spark/pull/39835
### What changes were proposed in this pull request?
Add UserDefinedType in protos, after this PR, `df.schema` will support UDT
note: UDT is still not supported in `df.collect` and `session.createDataFrame`, it needs changes in `ArrowConverters`
### Why are the changes needed?
for parity
### Does this PR introduce _any_ user-facing change?
yes
### How was this patch tested?
added tests
--
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 #39835: [SPARK-42268][CONNECT][PYTHON] Add UserDefinedType in protos
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #39835:
URL: https://github.com/apache/spark/pull/39835#discussion_r1094216810
##########
connector/connect/common/src/main/protobuf/spark/connect/types.proto:
##########
@@ -172,4 +175,12 @@ message DataType {
bool value_contains_null = 3;
uint32 type_variation_reference = 4;
}
+
+ message UDT {
+ string type = 1;
+ optional string jvm_class = 2;
+ optional string python_class = 3;
+ optional string serialized_python_class = 4;
+ DataType sql_type = 5;
Review Comment:
names are `jvm_class` for Scala UDT and `python_class` for Python UDT;
serialized data is `serialized_python_class` it use `string` instead of `bytes` since the [`serializedPyClass`](https://github.com/apache/spark/blob/1a42aa5bd44e7524bb55463bbd85bea782715834/sql/catalyst/src/main/scala/org/apache/spark/sql/types/UserDefinedType.scala#L51) is string when `UserDefinedType` was implemented.
--
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] xinrong-meng commented on a diff in pull request #39835: [SPARK-42268][CONNECT][PYTHON] Add UserDefinedType in protos
Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng commented on code in PR #39835:
URL: https://github.com/apache/spark/pull/39835#discussion_r1092796301
##########
connector/connect/server/src/main/scala/org/apache/spark/sql/connect/planner/DataTypeProtoConverter.scala:
##########
@@ -114,6 +118,31 @@ object DataTypeProtoConverter {
MapType(toCatalystType(t.getKeyType), toCatalystType(t.getValueType), t.getValueContainsNull)
}
+ private def toCatalystUDT(t: proto.DataType.UDT): UserDefinedType[_] = {
+ if (t.getType != "udf") {
Review Comment:
Should it be `udt`?
--
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] xinrong-meng commented on a diff in pull request #39835: [SPARK-42268][CONNECT][PYTHON] Add UserDefinedType in protos
Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng commented on code in PR #39835:
URL: https://github.com/apache/spark/pull/39835#discussion_r1092797190
##########
python/pyspark/sql/connect/types.py:
##########
@@ -184,6 +196,14 @@ def proto_schema_to_pyspark_data_type(schema: pb2.DataType) -> DataType:
proto_schema_to_pyspark_data_type(schema.map.value_type),
schema.map.value_contains_null,
)
+ elif schema.HasField("udt"):
+ assert schema.udt.type == "udt"
+ json_value = {}
+ if schema.udt.HasField("python_class"):
+ json_value["pyClass"] = schema.udt.python_class
+ if schema.udt.HasField("python_class"):
Review Comment:
Shoud it be `serialized_python_class`?
--
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 #39835: [SPARK-42268][CONNECT][PYTHON] Add UserDefinedType in protos
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon closed pull request #39835: [SPARK-42268][CONNECT][PYTHON] Add UserDefinedType in protos
URL: https://github.com/apache/spark/pull/39835
--
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 #39835: [SPARK-42268][CONNECT][PYTHON] Add UserDefinedType in protos
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #39835:
URL: https://github.com/apache/spark/pull/39835#discussion_r1092840612
##########
connector/connect/server/src/main/scala/org/apache/spark/sql/connect/planner/DataTypeProtoConverter.scala:
##########
@@ -114,6 +118,31 @@ object DataTypeProtoConverter {
MapType(toCatalystType(t.getKeyType), toCatalystType(t.getValueType), t.getValueContainsNull)
}
+ private def toCatalystUDT(t: proto.DataType.UDT): UserDefinedType[_] = {
+ if (t.getType != "udf") {
Review Comment:
yes, right now this field in all UDTs is `udt`.
but this field is only used in JSON, maybe able to git rid of it.
https://github.com/apache/spark/blob/1a42aa5bd44e7524bb55463bbd85bea782715834/sql/catalyst/src/main/scala/org/apache/spark/sql/types/UserDefinedType.scala#L61-L66
https://github.com/apache/spark/blob/1a42aa5bd44e7524bb55463bbd85bea782715834/sql/catalyst/src/main/scala/org/apache/spark/sql/types/UserDefinedType.scala#L127-L132
https://github.com/apache/spark/blob/1a42aa5bd44e7524bb55463bbd85bea782715834/python/pyspark/sql/types.py#L731-L749
--
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 #39835: [SPARK-42268][CONNECT][PYTHON] Add UserDefinedType in protos
Posted by "grundprinzip (via GitHub)" <gi...@apache.org>.
grundprinzip commented on code in PR #39835:
URL: https://github.com/apache/spark/pull/39835#discussion_r1094208220
##########
connector/connect/common/src/main/protobuf/spark/connect/types.proto:
##########
@@ -172,4 +175,12 @@ message DataType {
bool value_contains_null = 3;
uint32 type_variation_reference = 4;
}
+
+ message UDT {
+ string type = 1;
+ optional string jvm_class = 2;
+ optional string python_class = 3;
+ optional string serialized_python_class = 4;
+ DataType sql_type = 5;
Review Comment:
@HyukjinKwon @zhengruifeng Quick question here, are these class names or serialized data? If data why is this not a `bytes` field?
--
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] xinrong-meng commented on a diff in pull request #39835: [SPARK-42268][CONNECT][PYTHON] Add UserDefinedType in protos
Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng commented on code in PR #39835:
URL: https://github.com/apache/spark/pull/39835#discussion_r1093083003
##########
python/pyspark/sql/connect/types.py:
##########
@@ -108,6 +109,17 @@ def pyspark_types_to_proto_types(data_type: DataType) -> pb2.DataType:
elif isinstance(data_type, ArrayType):
ret.array.element_type.CopyFrom(pyspark_types_to_proto_types(data_type.elementType))
ret.array.contains_null = data_type.containsNull
+ elif isinstance(data_type, UserDefinedType):
+ json_value = data_type.jsonValue()
+ ret.udt.type = "udt"
+ if "class" in json_value:
+ # Scala/Java UDT
+ ret.udt.jvm_class = json_value["class"]
+ else:
+ # Python UDT
+ ret.udt.serialized_python_class = json_value["serializedClass"]
+ ret.udt.python_class = json_value["pyClass"]
Review Comment:
Makes sense, thanks!
--
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 #39835: [SPARK-42268][CONNECT][PYTHON] Add UserDefinedType in protos
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #39835:
URL: https://github.com/apache/spark/pull/39835#discussion_r1092841618
##########
python/pyspark/sql/connect/types.py:
##########
@@ -184,6 +196,14 @@ def proto_schema_to_pyspark_data_type(schema: pb2.DataType) -> DataType:
proto_schema_to_pyspark_data_type(schema.map.value_type),
schema.map.value_contains_null,
)
+ elif schema.HasField("udt"):
+ assert schema.udt.type == "udt"
+ json_value = {}
+ if schema.udt.HasField("python_class"):
+ json_value["pyClass"] = schema.udt.python_class
+ if schema.udt.HasField("python_class"):
Review Comment:
good catch!
--
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 #39835: [SPARK-42268][CONNECT][PYTHON] Add UserDefinedType in protos
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #39835:
URL: https://github.com/apache/spark/pull/39835#discussion_r1092847784
##########
python/pyspark/sql/connect/types.py:
##########
@@ -108,6 +109,17 @@ def pyspark_types_to_proto_types(data_type: DataType) -> pb2.DataType:
elif isinstance(data_type, ArrayType):
ret.array.element_type.CopyFrom(pyspark_types_to_proto_types(data_type.elementType))
ret.array.contains_null = data_type.containsNull
+ elif isinstance(data_type, UserDefinedType):
+ json_value = data_type.jsonValue()
+ ret.udt.type = "udt"
+ if "class" in json_value:
+ # Scala/Java UDT
+ ret.udt.jvm_class = json_value["class"]
+ else:
+ # Python UDT
+ ret.udt.serialized_python_class = json_value["serializedClass"]
+ ret.udt.python_class = json_value["pyClass"]
Review Comment:
refer to the `jsonValue` and `fromJson` methods in Python, the `pyClass` is a required field for Python UDT, otherwise it can not ser/deser in JSON format.
https://github.com/apache/spark/blob/51b04406028e14fbe1986f6a3ffc67853bd82935/python/pyspark/sql/types.py#L1098-L1130
--
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] xinrong-meng commented on a diff in pull request #39835: [SPARK-42268][CONNECT][PYTHON] Add UserDefinedType in protos
Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng commented on code in PR #39835:
URL: https://github.com/apache/spark/pull/39835#discussion_r1092801467
##########
python/pyspark/sql/connect/types.py:
##########
@@ -108,6 +109,17 @@ def pyspark_types_to_proto_types(data_type: DataType) -> pb2.DataType:
elif isinstance(data_type, ArrayType):
ret.array.element_type.CopyFrom(pyspark_types_to_proto_types(data_type.elementType))
ret.array.contains_null = data_type.containsNull
+ elif isinstance(data_type, UserDefinedType):
+ json_value = data_type.jsonValue()
+ ret.udt.type = "udt"
+ if "class" in json_value:
+ # Scala/Java UDT
+ ret.udt.jvm_class = json_value["class"]
+ else:
+ # Python UDT
+ ret.udt.serialized_python_class = json_value["serializedClass"]
+ ret.udt.python_class = json_value["pyClass"]
Review Comment:
Out of curiosity, why should a Scala/Java UDT have "pyClass"?
--
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] xinrong-meng commented on a diff in pull request #39835: [SPARK-42268][CONNECT][PYTHON] Add UserDefinedType in protos
Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng commented on code in PR #39835:
URL: https://github.com/apache/spark/pull/39835#discussion_r1093072128
##########
connector/connect/server/src/main/scala/org/apache/spark/sql/connect/planner/DataTypeProtoConverter.scala:
##########
@@ -114,6 +118,31 @@ object DataTypeProtoConverter {
MapType(toCatalystType(t.getKeyType), toCatalystType(t.getValueType), t.getValueContainsNull)
}
+ private def toCatalystUDT(t: proto.DataType.UDT): UserDefinedType[_] = {
+ if (t.getType != "udf") {
Review Comment:
Thanks!
--
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 #39835: [SPARK-42268][CONNECT][PYTHON] Add UserDefinedType in protos
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #39835:
URL: https://github.com/apache/spark/pull/39835#issuecomment-1413029814
Merged to master and branch-3.4.
--
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 #39835: [SPARK-42268][CONNECT][PYTHON] Add UserDefinedType in protos
Posted by "grundprinzip (via GitHub)" <gi...@apache.org>.
grundprinzip commented on code in PR #39835:
URL: https://github.com/apache/spark/pull/39835#discussion_r1094208897
##########
connector/connect/common/src/main/protobuf/spark/connect/types.proto:
##########
@@ -172,4 +175,12 @@ message DataType {
bool value_contains_null = 3;
uint32 type_variation_reference = 4;
}
+
+ message UDT {
+ string type = 1;
+ optional string jvm_class = 2;
+ optional string python_class = 3;
+ optional string serialized_python_class = 4;
+ DataType sql_type = 5;
Review Comment:
If it's name, the field name should indicate that.
--
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] xinrong-meng commented on a diff in pull request #39835: [SPARK-42268][CONNECT][PYTHON] Add UserDefinedType in protos
Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng commented on code in PR #39835:
URL: https://github.com/apache/spark/pull/39835#discussion_r1092797190
##########
python/pyspark/sql/connect/types.py:
##########
@@ -184,6 +196,14 @@ def proto_schema_to_pyspark_data_type(schema: pb2.DataType) -> DataType:
proto_schema_to_pyspark_data_type(schema.map.value_type),
schema.map.value_contains_null,
)
+ elif schema.HasField("udt"):
+ assert schema.udt.type == "udt"
+ json_value = {}
+ if schema.udt.HasField("python_class"):
+ json_value["pyClass"] = schema.udt.python_class
+ if schema.udt.HasField("python_class"):
Review Comment:
nit: Duplicate `if`s?
--
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