[GitHub] [spark] pang-wu commented on a diff in pull request #40686: [SPARK-43051][CONNECTOR] Add option to materialize zero values for fields without presence information (proto3 scalars)

["pang-wu (via GitHub)" <gi...@apache.org>]

pang-wu commented on code in PR #40686:
URL: https://github.com/apache/spark/pull/40686#discussion_r1181159725


##########
connector/protobuf/src/main/scala/org/apache/spark/sql/protobuf/utils/ProtobufOptions.scala:
##########
@@ -46,6 +46,41 @@ private[sql] class ProtobufOptions(
   // record has more depth than the allowed value for recursive fields, it will be truncated
   // and corresponding fields are ignored (dropped).
   val recursiveFieldMaxDepth: Int = parameters.getOrElse("recursive.fields.max.depth", "-1").toInt
+
+  // Whether to deserialize empty proto3 scalar fields as their default values.
+  // When a proto3 scalar field is empty, there is ambiguity as to whether the field
+  // was never set or if the field was explicitly set to zero. By default, the library

Review Comment:
   You are right this feature does not solve the ambiguity, but rather bring the behavior align with the proto3 spec (unset field set as 0/default value in this struct). 
   
   As long as the message is not created using proto2 syntax or with optional key word supported since protobuf 3.15, there is no way the ambiguity could be solved. 
   
   Maybe we can be explicit about that in the comment, I will work with @justaparth on 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