randreucetti opened a new pull request, #18434:
URL: https://github.com/apache/iceberg/pull/18434

   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](https://claude.com/claude-code)
   


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