justaparth commented on code in PR #41075:
URL: https://github.com/apache/spark/pull/41075#discussion_r1186875033
##########
connector/protobuf/src/test/scala/org/apache/spark/sql/protobuf/ProtobufFunctionsSuite.scala:
##########
@@ -361,15 +361,23 @@ class ProtobufFunctionsSuite extends QueryTest with
SharedSparkSession with Prot
val df = Seq(dynamicMessage.toByteArray).toDF("value")
+ // Test that roundtrip serde works correctly both with and without enums
as ints.
checkWithFileAndClassName("SimpleMessageEnum") {
case (name, descFilePathOpt) =>
- val fromProtoDF = df.select(
- from_protobuf_wrapper($"value", name,
descFilePathOpt).as("value_from"))
- val toProtoDF = fromProtoDF.select(
- to_protobuf_wrapper($"value_from", name,
descFilePathOpt).as("value_to"))
- val toFromProtoDF = toProtoDF.select(
- from_protobuf_wrapper($"value_to", name,
descFilePathOpt).as("value_to_from"))
- checkAnswer(fromProtoDF.select($"value_from.*"),
toFromProtoDF.select($"value_to_from.*"))
+ List(
+ Map.empty[String, String],
+ Map("enums.as.ints" -> "false"),
+ Map("enums.as.ints" -> "true"))
+ .foreach(opts => {
+ val fromProtoDF = df.select(
+ from_protobuf_wrapper($"value", name, descFilePathOpt,
opts).as("value_from"))
+ val toProtoDF = fromProtoDF.select(
+ to_protobuf_wrapper($"value_from", name,
descFilePathOpt).as("value_to"))
Review Comment:
yeah, i'm not sure either and also don't have a very strong opinion.
It felt safe to allow it by default because its more permissive, and also
was hard for me to imagine how it could go wrong (if you have int values in
your struct and you try to serialize and they're valid in the enum, then
_probably_ thats what you were intending to do).
And this isn't _necessarily_ a good argument for what we should , but it
appears that `JsonFormat`, for example, does this by default as well:
https://github.com/protocolbuffers/protobuf/blob/main/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java#L1920C33-L1946
So i'm inclined to leave it available by default, but yeah I don't really
have a strong opinion either way!
--
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]
