comphead opened a new pull request, #7603:
URL: https://github.com/apache/datafusion-comet/pull/7603

   ## Which issue does this PR close?
   
   Closes #6955.
   
   Part of #6957: this PR covers its first part, the S3 settings that cannot 
change the written object, where it is written or who writes it. Credentials 
and encryption follow in a separate PR.
   
   ## Rationale for this change
   
   The native Iceberg write gate declines several settings that cannot change 
what iceberg-java writes. Clusters commonly set them for every job, so on those 
clusters every Iceberg write falls back to iceberg-java.
   
   ## What changes are included in this PR?
   
   - **`write.parquet.shred-variants`** is no longer gated. Iceberg 1.11's 
`ParquetFormatModel` shreds only top-level `variant` columns, and 
`requireSupportedColumnTypes` already declines a `variant` column at any depth. 
Spark 4.x on Iceberg 1.11 resolves the property into every Parquet write, so a 
session that turned shredding on made every write fall back.
   - **An `io-impl` table property** is no longer gated. Every Iceberg catalog 
reads `io-impl` from catalog properties only (checked in the 1.5.2 and 1.11.0 
sources). `requireRecognizedTableFileIO` still checks the FileIO the table uses.
   - **`parquet.*` keys.** iceberg-java's writer copies every table and write 
property into its Hadoop configuration, but builds `ParquetProperties` itself. 
The only `parquet.*` keys that reach the written bytes are 
`parquet.columnindex.truncate.length` (Iceberg's `ParquetWriter`) and 
`parquet.compression.codec.zstd.level` and `.workers` (parquet-mr's 
`ZstandardCodec`, through Iceberg's `ParquetCodecFactory`). 
`ParquetKeysReadByIcebergWriter` gates those three, in table and write 
properties and in the session and `HadoopFileIO` Hadoop configurations. 
`parquet.enable.dictionary` as a table or write property keeps its own rule 
(#6954). Every other `parquet.*` key is ignored.
     - This checks `parquet.*` the other way round from the issue's proposal, 
which was an allowlist built from `ParquetOutputFormat` and 
`ParquetInputFormat` constants that fails closed on other keys. Such a list 
would still decline keys that no parquet-mr version declares, such as vendor 
reader flags, and `parquet.hadoop.vectored.io.enabled` on parquet-mr releases 
before 1.15. The tradeoff is that a codec key a future parquet-mr adds is 
admitted until the list is re-audited. The comment, the contributor guide and 
the review skill now ask for that re-audit on every Iceberg or parquet-mr bump.
     - The `HadoopFileIO` configuration is new to this check. It carries the 
catalog's `hadoop.*` overrides, which reach iceberg-java's writer but not the 
session configuration.
   - **S3A settings** that cannot change the written object, where it is 
written or who writes it are ignored (`IgnoredHadoopS3Suffixes`, audited 
against `org.apache.hadoop.fs.s3a.Constants` in hadoop-aws 3.3.4 and 3.4.2):
     - connection pool, threads, timeouts, retries, upload buffering and 
multipart sizing
     - read, listing and delete settings
     - `fs.s3a.impl` when it names `S3AFileSystem`
     - `fs.s3a.region`, which no hadoop-aws release from 3.3.4 to 3.5.0 reads
   
     The data bucket's per-bucket spellings are now ignored too, including 
those of the previously ignored committer and read settings. Endpoint and 
region selection, SSL, proxies, signing, requester pays, ACLs, storage class, 
checksums, headers, encryption, access points and credentials still fall back.
   - **`S3FileIO` settings** are ignored likewise: multipart, staging 
directory, retries, delete batching and client preloading. `s3.delete-enabled` 
is admitted only when it is `true`, because a failed native write task deletes 
its files, which `S3FileIO` keeps when deletes are disabled.
   - User guide, contributor guide and the Iceberg write review skill are 
updated to match.
   
   ## How are these changes tested?
   
   `CometIcebergWriteDetectionSuite`:
   
   - Flipped cases: `write.parquet.shred-variants=true`, an `io-impl` table 
property, and `parquet.block.size` in the session Hadoop configuration are now 
`Compatible`.
   - New `parquet.*` cases:
     - a `variant` column with shredding on still falls back on its type
     - ignored `parquet.*` keys in table properties and in the session 
configuration, including one parquet-mr does not declare
     - each of the three read keys in the session configuration falls back
     - a catalog `hadoop.*` override of a read key falls back
   - New S3 cases:
     - S3A transport settings and their per-bucket forms
     - `fs.s3a.impl` naming another class still falls back
     - `S3FileIO` transport settings, and `s3.delete-enabled=false`
     - an S3 write planned under the cluster-wide S3A and `parquet.*` settings 
above is `Compatible`
   


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