You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "rangadi (via GitHub)" <gi...@apache.org> on 2023/05/02 22:15:56 UTC

[GitHub] [spark] rangadi commented on a diff in pull request #40686: [SPARK-43051][CONNECT] Add option to emit default values

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


##########
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 render fields with zero values when deserializing protobufs to a spark struct.

Review Comment:
   Fix capitalization throughout this. E.g. `Spark` instead of `spark`, `Protobuf` instead of `protobuf`.  



##########
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 render fields with zero values when deserializing protobufs to a spark struct.
+  // When a field is empty in the serialized protobuf, this library will deserialize them as
+  // null by default. However, this flag can control whether to render the type-specific zero value.
+  // This operates similarly to `includingDefaultValues` in java's jsonformat, or `emitDefaults` in

Review Comment:
   nit: protobuf-java-util's rather than java's? it is not Java library. 
   Also, `JsonFormat` (with capital letter). 
   
   Alternately we could say `com.google.protobuf.util.JsonFormat` and avoid saying `java's jsonformat`. 



-- 
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