rdblue commented on code in PR #12847:
URL: https://github.com/apache/iceberg/pull/12847#discussion_r2062717955
##########
parquet/src/test/java/org/apache/iceberg/parquet/TestVariantWriters.java:
##########
@@ -104,6 +137,11 @@ public class TestVariantWriters {
Variant.of(EMPTY_METADATA, EMPTY_OBJECT),
Variant.of(TEST_METADATA, TEST_OBJECT),
Variant.of(TEST_METADATA, SIMILAR_OBJECT),
+ Variant.of(TEST_METADATA, ARRAY_IN_OBJECT),
+ Variant.of(EMPTY_METADATA, EMPTY_ARRAY),
+ Variant.of(EMPTY_METADATA, TEST_ARRAY),
+ Variant.of(EMPTY_METADATA, TEST_NESTED_ARRAY),
+ Variant.of(TEST_METADATA, TEST_OBJECT_IN_ARRAY),
Review Comment:
This test uses `ParquetVariantUtil.ParquetSchemaProducer` to produce a
shredding schema, but that hasn't been updated to support arrays so the
shredded case for arrays is not testing the new shredded writer. You can check
by running the tests with coverage.
I think it would be a good idea to update `ParquetSchemaProducer` to shred
an array if it has a consistent element type across all elements. Then you'd
need to ensure that the test cases here exercise that path by making some of
the arrays have one element or a consistent type across elements.
--
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]
