amogh-jahagirdar commented on code in PR #18434:
URL: https://github.com/apache/iceberg/pull/18434#discussion_r4242912557


##########
arrow/src/test/java/org/apache/iceberg/arrow/vectorized/TestVectorizedReaderBuilder.java:
##########
@@ -70,6 +72,41 @@ public void testVariantSkippedWhenNotInProjection() {
             () -> TypeWithSchemaVisitor.visit(icebergSchema.asStruct(), 
parquetSchema, builder));
   }
 
+  @Test
+  public void testShreddedVariantSkippedWhenNotInProjection() {
+    Schema icebergSchema = new Schema(NestedField.required(1, "id", 
IntegerType.get()));
+
+    MessageType parquetSchema = parquetSchemaWithShreddedVariant();
+
+    VectorizedReaderBuilder builder =
+        new VectorizedReaderBuilder(
+            icebergSchema, parquetSchema, false, ImmutableMap.of(), readers -> 
null);
+
+    assertThatNoException()
+        .describedAs("Shredded variant not in projection should not throw")
+        .isThrownBy(
+            () -> TypeWithSchemaVisitor.visit(icebergSchema.asStruct(), 
parquetSchema, builder));
+  }
+
+  @Test
+  public void testShreddedVariantInProjectionThrows() {

Review Comment:
   I think the practice for new tests is to just drop the test prefix in the 
method name; I think something like `projectingShreddedVariantUnsupported` is 
also a bit more clear (mostly the "Unsupported" part, the "throws" in the 
current naming is a bit vague imo)



##########
arrow/src/test/java/org/apache/iceberg/arrow/vectorized/TestVectorizedReaderBuilder.java:
##########
@@ -70,6 +72,41 @@ public void testVariantSkippedWhenNotInProjection() {
             () -> TypeWithSchemaVisitor.visit(icebergSchema.asStruct(), 
parquetSchema, builder));
   }
 
+  @Test
+  public void testShreddedVariantSkippedWhenNotInProjection() {

Review Comment:
   I think it's adding some tests for when the variant is nested inside struct, 
as a list element, map key/value and isn't projected. 



-- 
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]

Reply via email to