andygrove opened a new pull request, #6149: URL: https://github.com/apache/datafusion-comet/pull/6149
## Which issue does this PR close? No issue. This is a contributor documentation and agent-tooling change, part of the native Iceberg writes epic #5649. ## Rationale for this change Iceberg writes have no contributor-guide page and no area review skill. The only description of the implementation is the user guide's `iceberg-writes.md`, which is written for users (configuration, eligibility, accepted differences from iceberg-java) and does not explain the code: how the split plan is built, what the eligibility gate must guarantee, what crosses the JVM/native boundary, or who owns cleanup when a task fails. That gap showed up in the bugs. Several recent write-path issues share a small set of causes that are easy to repeat and hard to spot in review: - a planning gate that admits something the native side then rejects, so the query fails instead of falling back (#6140) - settings the native writer silently ignores (#5637, #6139) - iceberg-rust value semantics that differ from iceberg-java's (#6138) - map iteration order reaching the output (#5776) - `void` partition fields left behind by partition evolution (#5691, #5693, #6141) - plan rewrites that lose the write node (#5719) - behaviour that only appears with AQE off (#5689) Writing these down once, next to the code they apply to, should help contributors and reviewers catch them before merge. This follows the shape of #6018 and #6052: an area review skill that anchors on a contributor-guide page for its subsystem. ## What changes are included in this PR? **New: `docs/source/contributor-guide/iceberg-writes.md`** Organized around one rule: the native writer produces what iceberg-java would have produced, or declines at plan time. It covers: - the split-operator plan (strategy, logical anchor, writer and committer, version shims) and the contracts between its operators - how `IcebergWriteExec` becomes `CometIcebergWriteExec`, including why a local-relation input needs `localTableScan` and how the input transition is stripped - the properties every eligibility rule must keep: allow list, effective configuration, instantiated state, fail closed, executor reflection probed on the driver, and agreement with the native side - the `IcebergWrite` proto, the native writer's adaptations to match iceberg-java, and the two-column task payload with the JVM-side metrics rebuild - a cleanup ownership table for each failure phase - parity tiers, the iceberg-rust pin, the test suites, and how to write a native-write test - a pitfalls list, each item linked to the issue where it caused a bug It links to the user guide for the eligibility table and the divergence list rather than repeating them. **New: `.ai/skills/review-comet-iceberg-write-pr/SKILL.md`** An area review skill in the shape of the existing siblings. It starts from the direction of a gate change: narrowing costs performance, while widening means Comet now claims writes it refused before, and passing tests are not evidence for that because they were written against the narrower gate. Checklists follow for output parity, cleanup ownership, plan shape (AQE off, transitions, exactly-once commit, the kill switch, credentials in plan strings), the iceberg-rust pin, tests that prove the native writer ran, and which docs a change can make stale. **Updated** - `review-comet-pr`: a routing row for the new skill, and its description lists the new area. - `contributor-guide/index.md`: a toctree entry under Project Architecture. #6052 also edits the `review-comet-pr` routing table and the contributor-guide toctree, so whichever merges second has a small conflict to resolve. ## How are these changes tested? Documentation only. `prettier --check` passes on the changed files, and a Sphinx build renders the new page with no new warnings. Class, method, and file names, proto fields, the cleanup mechanics, and the Iceberg version pinned per Spark profile were checked against the source on `main`. -- 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]
