andygrove opened a new pull request, #6672: URL: https://github.com/apache/datafusion-comet/pull/6672
## Which issue does this PR close? Closes #6668. ## Rationale for this change Every write to a format-version 3 table falls back to iceberg-java today. In the Iceberg 1.11 coverage run of 2026-10-04 that was 1452 writes, the third-largest fallback bucket. The only thing iceberg-java writes differently for a v3 table is row lineage, and it writes lineage columns only when it rewrites existing rows: copy-on-write `DELETE`, `UPDATE` and `MERGE` and `rewrite_data_files`, from Iceberg 1.10. Appends and overwrites write the data columns alone, and the driver gives their rows ids at commit time, on the commit path the native writer already uses. Iceberg 1.8 writes no lineage columns at all. ## What changes are included in this PR? `CometIcebergNativeWrite`: - Admits `format-version` 3. Format version 4, which is still being specified, keeps falling back. - New `requireNoMetadataColumns` rule: a write whose write schema has a field id above `Integer.MAX_VALUE - 200`, the range the spec reserves for metadata columns, falls back. On Iceberg 1.10+ that is a v3 copy-on-write `DELETE`/`UPDATE`/`MERGE` or `rewrite_data_files`, whose write schema includes `_row_id` and `_last_updated_sequence_number`. Matching the id range rather than the names covers every Iceberg version and needs no new reflection target. - The `uuid` column-type rule also declines the v3-only types `variant`, `unknown`, `timestamp_ns`, `geometry` and `geography`, which the version rule used to keep out. `IcebergReflection.getSchemaFieldIds` backs the new rule, and `getSchemaFieldNames` now derives from it. Comment fixes. The scaladoc on `newDataManifestFile` said the 3-arg `GenericManifestFile` constructor takes `firstRowId`. It takes the snapshot id and leaves `first_row_id` null, which is why the decoded `DataFile`s carry no row ids and the v3 commit assigns them. `dropNonDataColumns` and `iceberg_partition_path.rs` pointed at the old version rule. Docs: the user guide's eligibility table and a paragraph on which v3 writes fall back, and the contributor guide's notes on `dropNonDataColumns` and row-id assignment. One consequence for reviewers to weigh: on a v3 table the order of data files in a commit decides which `_row_id` each file's rows get. A fanout write lists a task's files in path order where iceberg-java uses its `StructLikeMap` order (an existing accepted divergence), so the same rows can get different ids from the two writers. The ids stay unique, and iceberg-java's own assignment across tasks already depends on task completion order. I added this to the fanout entry in the accepted divergences rather than gating fanout writes on v3. Writing row lineage natively, so v3 copy-on-write and compaction can run natively too, is #6669. ## How are these changes tested? `CometIcebergWriteDetectionSuite`: a v3 append is `Compatible`, and `format-version=4`, a `variant` column and an `unknown` column each fall back with a specific reason. `CometIcebergWriteActionSuite`, each test comparing a natively written table with a sibling written by iceberg-java: - A v3 append and dynamic overwrite across eight partitions: rows, partition directories, per-partition `readable_metrics`, and on Iceberg 1.10+ each row's `_last_updated_sequence_number` and which commit its `_row_id` came from. - v3 row lineage with one file per commit: exact `_row_id` and `_last_updated_sequence_number` after two native appends, then a copy-on-write `UPDATE` that stays on iceberg-java with a fallback reason naming `_row_id` and leaves the updated row with its id and the new sequence number. - A v3 column with initial and write defaults, inserted with every column, and on Iceberg 1.11 with the column omitted so Spark fills in the default. Iceberg 1.10 does not expose defaults to Spark, which rejects that insert. `CometIcebergRewriteActionSuite`: a v3 table built by four native appends has the expected row ids, and `rewrite_data_files` keeps them. The rewrite stays on iceberg-java on Iceberg 1.10+ and runs natively on 1.8. Ran locally. On Spark 4.1 / Iceberg 1.11, all of `CometIcebergWriteDetectionSuite`, `CometIcebergWriteActionSuite`, `CometIcebergRewriteActionSuite` and `CometIcebergSystemFunctionSuite` pass (174 tests). On Spark 3.5 / Iceberg 1.8.1 and Spark 4.0 / Iceberg 1.10.0, `CometIcebergWriteDetectionSuite` and the new action and rewrite tests pass; on 1.8.1 the tests that need Iceberg 1.10 are skipped. Iceberg's own Spark tests make about 1450 v3 writes that can now go native, so this should get the `run-iceberg-tests` label. -- 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]
