andygrove opened a new pull request, #6996:
URL: https://github.com/apache/datafusion-comet/pull/6996
## Which issue does this PR close?
Closes #6960.
## Rationale for this change
parquet-mr compresses and decompresses brotli pages with Hadoop's
`org.apache.hadoop.io.compress.BrotliCodec`, which it loads by name. Neither
Spark nor Iceberg
ships that class, and the Iceberg runtime's bundled copy of Parquet looks up
the same one. Without
a brotli codec jar, iceberg-java fails a brotli write. The eligibility gate
admitted brotli
anyway, and parquet-rs compresses it without the class, so a native write
committed a table that
Spark without Comet cannot read. It fails with
`BadConfigurationException: Class org.apache.hadoop.io.compress.BrotliCodec
was not found`, and so
would Comet whenever its scan falls back to Spark's reader.
#6960 also reported that iceberg-java fails a gzip write with a numeric
`write.parquet.compression-level`, which the native writer accepts. That
case is different: Hadoop
reads `zlib.compress.level` as the name of a
`ZlibCompressor.CompressionLevel` constant, so `5`
throws, but the native files are ordinary gzip pages that every reader can
read. Spark 4.1.1's
parquet-mr reads a gzip level 5 file without trouble. This PR documents that
difference instead of
making the native path fail as well.
## What changes are included in this PR?
- A new eligibility rule, `requireLoadableBrotliCodec`, declines a brotli
write unless
`org.apache.hadoop.io.compress.BrotliCodec` loads through the thread
context class loader, which
Spark adds application jars to. It loads the class without initializing
it, so the check runs no
codec code. It reads the effective codec, so
`spark.sql.iceberg.compression-codec` and the
`compression-codec` write option count too. It runs after the compression
level rule, so an
out-of-range brotli level still reports the level.
- `IcebergWriteProtoTranslation.compressesWithBrotli` reads the codec name
as `resolveCompression`
does, but never throws, so an unknown codec such as `lzo` still fails
where it did before.
- User guide (`iceberg-writes.md`): the eligibility table's compression row
and a new paragraph
describe the brotli rule, and the accepted-divergences entry on codec
settings now covers a
numeric gzip level. The table cell keeps its width, so the rest of the
table is not re-padded.
#6961 adds a list of plans for each fallback to the same page. Whichever of
the two merges second
should add this rule to its "Not planned" list, beside the other writes
iceberg-java itself
rejects.
## How are these changes tested?
A new `CometIcebergWriteDetectionSuite` case plans two brotli writes without
running them, one
set by a table property and one by `spark.sql.iceberg.compression-codec`,
and checks that each
falls back with the new reason. Under a context class loader that resolves
the class name, as one
with a brotli codec jar would, both are `Compatible`. Breaking the rule
either way, never declining
or always declining, fails the test.
Locally, `CometIcebergWriteDetectionSuite` and
`IcebergWriteProtoTranslationSuite` pass on the
default Spark 4.1 profile (104 tests) and on the Spark 3.4 profile with
Iceberg 1.5.2 (100 passed,
4 canceled by their version checks).
An end-to-end check, not committed, ran a brotli and a zstd `INSERT` with
`spark.comet.write.iceberg.enabled=true` and
`spark.comet.exec.localTableScan.enabled=true`:
- With the rule disabled, as on `main`: the brotli plan shows
`CometIcebergWrite` and the insert
succeeds, and reading the table with `spark.comet.enabled=false` fails with
`BadConfigurationException: Class
org.apache.hadoop.io.compress.BrotliCodec was not found`.
- With this PR: the brotli plan shows `IcebergWrite`, and the insert fails
with that same error,
as it does without Comet.
- The zstd table is written natively in both runs, and Spark without Comet
reads it.
The Iceberg Spark test suites are unaffected: none of them writes brotli
(checked on a recent
Iceberg `main`), and Iceberg's build adds no brotli codec.
--
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]
