rangadi commented on code in PR #40686:
URL: https://github.com/apache/spark/pull/40686#discussion_r1181137367
##########
connector/protobuf/src/main/scala/org/apache/spark/sql/protobuf/ProtobufDeserializer.scala:
##########
@@ -299,6 +298,43 @@ private[sql] class ProtobufDeserializer(
}
}
+ private def getFieldValue(record: DynamicMessage, field: FieldDescriptor):
AnyRef = {
+ // We return a value if one of:
+ // - the field is repeated
+ // - the field is explicitly present in the serialized proto
+ // - the field is proto2 with a default
+ // - we should materialize the zero value
+ //
+ // Repeated fields have to be treated separately as they cannot have
`hasField`
+ // called on them.
+ if (
+ field.isRepeated
+ || record.hasField(field) // checks for the field explicitly being set
+ || field.hasDefaultValue
+ || shouldGetZeroValue(record, field)) {
+ record.getField(field)
+ } else {
+ null
+ }
+ }
+
+ // If the materializeZeroValues config is set, we will materialize zero
values when the field
Review Comment:
Ideally we shouldn't need very long comment. Brief line and pointing to
documentation of the flag should be enough. with that, we could move the check
back into `getFieldValue()` method.
##########
connector/protobuf/src/main/scala/org/apache/spark/sql/protobuf/ProtobufDeserializer.scala:
##########
@@ -299,6 +298,43 @@ private[sql] class ProtobufDeserializer(
}
}
+ private def getFieldValue(record: DynamicMessage, field: FieldDescriptor):
AnyRef = {
+ // We return a value if one of:
+ // - the field is repeated
+ // - the field is explicitly present in the serialized proto
+ // - the field is proto2 with a default
+ // - we should materialize the zero value
+ //
+ // Repeated fields have to be treated separately as they cannot have
`hasField`
+ // called on them.
+ if (
+ field.isRepeated
+ || record.hasField(field) // checks for the field explicitly being set
+ || field.hasDefaultValue
+ || shouldGetZeroValue(record, field)) {
+ record.getField(field)
+ } else {
+ null
+ }
+ }
+
+ // If the materializeZeroValues config is set, we will materialize zero
values when the field
Review Comment:
Ideally we shouldn't need very long comment here again. Brief line and
pointing to documentation of the flag should be enough. with that, we could
move the check back into `getFieldValue()` method.
--
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]
