rdblue commented on code in PR #13219:
URL: https://github.com/apache/iceberg/pull/13219#discussion_r2232224562
##########
spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/data/GenericsHelpers.java:
##########
@@ -296,6 +320,134 @@ private static void assertEqualsUnsafe(Types.MapType map,
Map<?, ?> expected, Ma
}
}
+ static void assertEquals(Variant expected, VariantVal actual) {
+ VariantMetadata actualMetadata =
+
VariantMetadata.from(ByteBuffer.wrap(actual.getMetadata()).order(ByteOrder.LITTLE_ENDIAN));
+ VariantTestUtil.assertEqual(expected.metadata(), actualMetadata);
+
+ org.apache.spark.types.variant.Variant sparkVariant =
+ new org.apache.spark.types.variant.Variant(actual.getValue(),
actual.getMetadata());
+ assertEquals(expected.value(), sparkVariant);
+ }
+
+ static void assertEquals(VariantValue expected,
org.apache.spark.types.variant.Variant actual) {
Review Comment:
I don't think that this belongs here. The assertions here verify that two
representations are the same, not that the contents of those are the same. For
example, there are tests that generate Iceberg generics, write to Parquet, and
read using the Spark object model. For that validation, we want to make sure
that the value that Iceberg's implementation of the Spark object model is
producing data correctly. There is no need for a dependency on Spark's
interpretation of Variant bytes to do that.
A validation that Iceberg and Spark agree on what those bytes should be and
what they mean is a different validation.
Another way to think about this is that this will cause tests to fail if
Iceberg supports new Variant types before Spark does. That's not a dependency
that is needed for object model tests. We should definitely test that Spark
interprets Variant bytes from Iceberg the same way, but in a more focused test
suite that is just looking for incompatibilities.
--
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]
