wmoustafa commented on code in PR #6004:
URL: https://github.com/apache/iceberg/pull/6004#discussion_r1173315886
##########
data/src/test/java/org/apache/iceberg/data/DataTestHelpers.java:
##########
@@ -88,6 +90,16 @@ private static void assertEquals(Type type, Object expected,
Object actual) {
"Primitive value should be equal to expected for type " + type,
expected, actual);
break;
case FIXED:
+ // For fixed type, Iceberg actually represent value as Bytebuffer,
+ // but since RandomGenericData generates bytearray data for fixed type
+ // for it to be written to Avro, we add a conversion here to make the
+ // equality comparison consistent using bytearray
+ if (expected instanceof ByteBuffer) {
+ expected = ByteBuffers.toByteArray((ByteBuffer) expected);
+ }
+ if (actual instanceof ByteBuffer) {
+ actual = ByteBuffers.toByteArray((ByteBuffer) actual);
Review Comment:
Also to clarify where the dependency on `byte[]` is coming from, it actually
comes from `DataTestHelpers`. We can see
[here](https://github.com/apache/iceberg/blob/d04efee702fcdcdbe3659c12f7442f5000aa246a/data/src/test/java/org/apache/iceberg/data/DataTestHelpers.java#L90)
that the `FIXED` case fails the assertion if the data type is not `byte[]`.
Hence, there is a conflict between `DataTestHelpers` which uses `byte[]` and
`Literals` which uses `ByteBuffer`. Such a gap is bridged in
`SingleValueParser#toJson` in the current PR.
--
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]
