rangadi commented on code in PR #38515:
URL: https://github.com/apache/spark/pull/38515#discussion_r1015740385
##########
connector/protobuf/src/test/scala/org/apache/spark/sql/protobuf/ProtobufSerdeSuite.scala:
##########
@@ -177,6 +177,31 @@ class ProtobufSerdeSuite extends SharedSparkSession {
withFieldMatchType(Deserializer.create(CATALYST_STRUCT, protoNestedFile,
_))
}
+ test("raise cannot parse protobuf descriptor error") {
+ // passing serde_suite.proto instead serde_suite.desc
+ val testFileDesc = testFile("serde_suite.proto").replace("file:/", "/")
+ val e = intercept[AnalysisException] {
+ ProtobufUtils.buildDescriptor(testFileDesc, "FieldMissingInSQLRoot")
+ }
+
+ checkError(
+ exception = e,
+ errorClass = "CANNOT_PARSE_PROTOBUF_DESCRIPTOR",
+ parameters = Map("descFilePath" -> testFileDesc))
+ }
+
+ test("raise cannot construct protobuf descriptor error") {
+ val testFileDesc =
testFile("basicmessage_noimports.desc").replace("file:/", "/")
+ val e = intercept[AnalysisException] {
+ ProtobufUtils.parseFileDescriptorSet(testFileDesc)
Review Comment:
Thanks for adding the test.
Do we need to invoke this? Any query would raise this, right?
##########
connector/protobuf/src/test/scala/org/apache/spark/sql/protobuf/ProtobufCatalystDataConversionSuite.scala:
##########
@@ -123,16 +123,21 @@ class ProtobufCatalystDataConversionSuite
StringType -> ("StringMsg", ""))
testingTypes.foreach { dt =>
- val seed = 1 + scala.util.Random.nextInt((1024 - 1) + 1)
+ val seed = scala.util.Random.nextInt(RandomDataGenerator.MAX_STR_LEN)
test(s"single $dt with seed $seed") {
val (messageName, defaultValue) =
catalystTypesToProtoMessages(dt.fields(0).dataType)
val rand = new scala.util.Random(seed)
val generator = RandomDataGenerator.forType(dt, rand = rand).get
var data = generator()
- while (data.asInstanceOf[Row].get(0) == defaultValue) // Do not use
default values, since
- data = generator() // from_protobuf()
returns null in v3.
+ // Do not use default values, since from_protobuf() returns null in v3.
+ while (
+ data != null &&
Review Comment:
I see. Equals does not check the content on array.
Optional:
We could recduce `data.asInstanceOf` calls with `val data =
generator().asInstanceOf[Row]`.
Also could replace
> `data.asInstanceOf[Row].get(0).isInstanceOf[Array[Byte]]`
with
> `dt == BinaryType`
--
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-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
