Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SPARK-41015][SQL][PROTOBUF] UnitTest null check for data generator #38515

Closed
wants to merge 5 commits into from
Closed

[SPARK-41015][SQL][PROTOBUF] UnitTest null check for data generator #38515

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bcc333f
null check for data generator
SandishKumarHN Nov 5, 2022
d4e26b9
bug fix Map.empty to Map with Utest
SandishKumarHN Nov 5, 2022
f11b04d
review changes Utest for cannt construct proto desc
SandishKumarHN Nov 6, 2022
c9abb2d
review changes nit, utests changes
SandishKumarHN Nov 7, 2022
44e9209
review changes utest, empty protobuf
SandishKumarHN Nov 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions ...uf/src/test/scala/org/apache/spark/sql/protobuf/ProtobufCatalystDataConversionSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should just be data == null || data.asInstanceOf[Row].get(0) == defaultValue?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant, we don't need the array check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rangadi

  1. data.asInstanceOf[Row].get(0) == ByteString.empty().toByteArray
  2. data.asInstanceOf[Row].get(0) == Array.emptyByteArray
  3. data.asInstanceOf[Row].get(0) == ByteString.EMPTY.toByteArray
  4. data.asInstanceOf[Row].get(0) == "".getBytes
  5. data.asInstanceOf[Row].get(0).isInstanceOf[Array[Byte]] && data.asInstanceOf[Row].get(0).asInstanceOf[Array[Byte]].isEmpty

Except for (5), none of them worked. I'm printing under the conditions listed above.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

(data.asInstanceOf[Row].get(0) == defaultValue ||
(data.asInstanceOf[Row].get(0).isInstanceOf[Array[Byte]] &&
data.asInstanceOf[Row].get(0).asInstanceOf[Array[Byte]].isEmpty)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't ByteString.empty().toByteArray at line 122 already empty value?

data = generator()

val converter = CatalystTypeConverters.createToCatalystConverter(dt)
val input = Literal.create(converter(data), dt)
Expand Down