Arrow, Spark: Skip unprojected shredded variants in vectorized Parquet reads

[randreucetti]

Closes #18430

Rationale for this change

A vectorized Spark read of a Parquet file that contains a shredded variant column fails when the query doesn't project the variant:

java.lang.UnsupportedOperationException: Unsupported variant: shredded typed_value primitive
  at org.apache.iceberg.arrow.vectorized.VectorizedVariantVisitor.primitive(VectorizedVariantVisitor.java:97)
  ...
  at org.apache.iceberg.parquet.TypeWithSchemaVisitor.visitVariant(TypeWithSchemaVisitor.java:218)
  ...
  at org.apache.iceberg.spark.data.vectorized.VectorizedSparkParquetReaders.buildReader(VectorizedSparkParquetReaders.java:61)

SparkBatch only routes projected shredded variants to the row reader, so a query like SELECT sum(id) FROM t on a shredded table uses batch reads. VectorizedSparkParquetReaders.buildReader then visits the file schema, and TypeWithSchemaVisitor.visitVariant walks the unprojected variant group with VectorizedVariantVisitor, which throws on typed_value. The variant isn't read at all, so the throw is unnecessary.

What changes are included in this PR?

VectorizedReaderBuilder.variantVisitor() returns null when the current variant group isn't in the expected schema. TypeWithSchemaVisitor then calls variant(...) with a null result, so the column gets no reader. This matches how VectorizedReaderBuilder.primitive(...) already treats unprojected primitives.

The fix is in the shared arrow module, so it covers every Spark version that uses VectorizedReaderBuilder. Projected shredded variants are unchanged: they still rely on SparkBatch routing them to the row reader.

I kept the change in the vectorized builder and left TypeWithSchemaVisitor.visitVariant alone. Skipping the sub-visit there when the Iceberg type is null would also work, but that visitor is shared with the row readers and BaseParquetWriter, so changing it has a wider blast radius. Happy to move it if reviewers prefer.

Are these changes tested?

Yes:

• TestVectorizedReaderBuilder.testShreddedVariantSkippedWhenNotInProjection is the shredded counterpart of the existing testVariantSkippedWhenNotInProjection. That test only uses an unshredded variant, which is why it didn't catch this. Without the fix the new test fails with the UnsupportedOperationException above.
• TestVectorizedReaderBuilder.testShreddedVariantInProjectionThrows checks that the skip applies only to unprojected variants: a projected shredded variant still reaches VectorizedVariantVisitor.
• TestSparkVariantRead.testReadShreddedWithoutProjectingVariant (Spark 4.2) is an end-to-end regression test. It writes a shredded table, asserts the file has a typed_value subtree, and reads only id with vectorization on and off. Without the fix, the vectorized case fails.

The same Spark test applies unchanged to 4.0 and 4.1. I'm happy to add it here or in a follow-up backport PR, whichever reviewers prefer.

Are there any user-facing changes?

No API changes. Vectorized reads that don't project a shredded variant now succeed instead of failing.

AI disclosure

I used an AI coding assistant (Claude Code) to help trace the bug, draft the fix and scaffold the tests. I reproduced the bug, reviewed the change, and ran the tests and checks locally. The area I'd most like reviewers to check is the field lookup in variantVisitor(). It resolves the variant's field id from parquetSchema.getType(currentPath()), which relies on the builder's parquetSchema being the same id-assigned file schema that TypeWithSchemaVisitor is walking. That holds for VectorizedSparkParquetReaders.buildReader.

cc @nssalian

🤖 Generated with Claude Code

[nssalian]

CC: @RussellSpitzer @huaxingao to help since I can't get to it at the moment. Looks right at a high level but needs a closer review.

[amogh-jahagirdar]

Thank you @randreucetti I agree with the fix. It makes sense in variantVisitor() since the variant's children are visited before variant(...) is called, so I believe this is the only way to indicate to the caller that a variant that's not being projected shouldn't be visited.

Just had some comments on tests.

Separately, I wonder why the reader builders get the full file schema rather than the projection, which is applied later when the pages are read. Not blocking because looks like that's always been the case and we still fundamentally get the benefits of projection pushdown but feels like we could get it even earlier in this logic too.

[amogh-jahagirdar]

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)

[amogh-jahagirdar]

I think it's worth adding a parameterized test for when the variant is nested inside struct, as a list element, map key/value and isn't projected.