rangadi commented on code in PR #38922:
URL: https://github.com/apache/spark/pull/38922#discussion_r1041469181
##########
connector/protobuf/src/main/scala/org/apache/spark/sql/protobuf/ProtobufDeserializer.scala:
##########
@@ -157,6 +157,8 @@ private[sql] class ProtobufDeserializer(
case (null, NullType) => (updater, ordinal, _) =>
updater.setNullAt(ordinal)
+ case (MESSAGE, NullType) => (updater, ordinal, _) =>
updater.setNullAt(ordinal)
Review Comment:
What is this for? For handling limited recursion?
##########
connector/protobuf/src/test/scala/org/apache/spark/sql/protobuf/ProtobufFunctionsSuite.scala:
##########
@@ -693,4 +721,45 @@ class ProtobufFunctionsSuite extends QueryTest with
SharedSparkSession with Prot
errorClass = "CANNOT_CONSTRUCT_PROTOBUF_DESCRIPTOR",
parameters = Map("descFilePath" -> testFileDescriptor))
}
+
+ test("Unit tests for OneOf field support and recursion checks") {
Review Comment:
Lets separate these two into separate tests with separate protobuf message.
##########
connector/protobuf/src/test/resources/protobuf/functions_suite.proto:
##########
@@ -170,4 +170,41 @@ message timeStampMsg {
message durationMsg {
string key = 1;
Duration duration = 2;
-}
\ No newline at end of file
+}
+
+message OneOfEvent {
Review Comment:
Are you testing more OneOf and recusion in the same message? Could you split
them into separate messages?
##########
connector/protobuf/src/main/scala/org/apache/spark/sql/protobuf/utils/SchemaConverters.scala:
##########
@@ -92,9 +92,13 @@ object SchemaConverters {
MapType(keyType, valueType, valueContainsNull =
false).defaultConcreteType,
nullable = false))
case MESSAGE =>
+ // Stop recursion at the first level when a recursive field is
encountered.
+ // TODO: The user should be given the option to set the recursion
level to 1, 2, or 3
Review Comment:
Are you planning to add selectable recursion depth here or in a follow up?
##########
connector/protobuf/src/test/resources/protobuf/functions_suite.proto:
##########
@@ -170,4 +170,41 @@ message timeStampMsg {
message durationMsg {
string key = 1;
Duration duration = 2;
-}
\ No newline at end of file
+}
+
+message OneOfEvent {
+ string key = 1;
+ oneof payload {
Review Comment:
How do one-of fields look like in spark schema? Could you give an example? I
could not see the schema in the unit 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-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
