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]