chenhao-db commented on code in PR #43707:
URL: https://github.com/apache/spark/pull/43707#discussion_r1389786868
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/UnsafeRowWriterSuite.scala:
##########
@@ -61,4 +61,15 @@ class UnsafeRowWriterSuite extends SparkFunSuite {
rowWriter.write(1, interval)
assert(rowWriter.getRow.getInterval(1) === interval)
}
+
+ test("write and get variant through UnsafeRowWriter") {
+ val rowWriter = new UnsafeRowWriter(2)
+ rowWriter.resetRowWriter()
+ rowWriter.setNullAt(0)
+ assert(rowWriter.getRow.isNullAt(0))
+ assert(rowWriter.getRow.getVariant(0) === null)
+ val variant = new VariantVal(Array[Byte](1, 2, 3), Array[Byte](-1, -2, -3,
-4))
+ rowWriter.write(1, variant)
+ assert(rowWriter.getRow.getVariant(1).debugString() ==
variant.debugString())
Review Comment:
I don't think we should have them at this moment: the equivalence of
`VariantVal` is much more complicated than byte-to-byte comparison (i.e.,
different binary values can represent the same variant). It will be pretty
complex and will depend on the detailed encoding. It will be confusing if we
have `equals` and `hashCode` as byte-to-byte comparisons.
In my mind, when we are testing the result of `VariantVal` in the future, we
should use semantic equivalence rather than byte-to-byte comparison. This test
is really a special case because we want to verify we exactly pass the
`VariantVal` throughout the system. We can add something like `equalByBytes`,
but it doesn't make things any simpler, so I choose to compare the
`debugString`.
--
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]
