rangadi commented on code in PR #41498:
URL: https://github.com/apache/spark/pull/41498#discussion_r1222219521


##########
connector/protobuf/src/main/scala/org/apache/spark/sql/protobuf/ProtobufDeserializer.scala:
##########
@@ -247,12 +247,86 @@ private[sql] class ProtobufDeserializer(
           updater.setLong(ordinal, micros + 
TimeUnit.NANOSECONDS.toMicros(nanoSeconds))
 
       case (MESSAGE, StringType)
-          if protoType.getMessageType.getFullName == "google.protobuf.Any" =>
+        if protoType.getMessageType.getFullName == "google.protobuf.Any" =>
         (updater, ordinal, value) =>
           // Convert 'Any' protobuf message to JSON string.
           val jsonStr = jsonPrinter.print(value.asInstanceOf[DynamicMessage])
           updater.set(ordinal, UTF8String.fromString(jsonStr))
 
+      // Handle well known wrapper types. We unpack the value field instead of 
keeping

Review Comment:
   > This is the purpose of the wrapper types, not of this PR.
   
   Better for Spark to preserve the same information, right? 
   There is a reason why it was used in the the users's Protobuf message.. 
might be simpler to Spark to preserve that. 
   
   If the justification is just convenience, i am not sure we need to do that. 
   
   > I think that the behavior currently is inconsistent with the intention of 
the types /
   
   I don't quite see inconsistency. The original purpose in Protobuf is to 
distinguish unset vs default value (as you mentioned). Spark handling preserves 
that. 
   Just because of JSON serializer does this does not imply Spark should do. 
   How does Java API for this look like?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to