You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "u70b3 (via GitHub)" <gi...@apache.org> on 2026/09/29 02:13:15 UTC

[PR] fix(cow_rewrite): guard partition preservation and document rewrite() trade-offs [iceberg-rust]

u70b3 opened a new pull request, #3296:
URL: https://github.com/apache/iceberg-rust/pull/3296

   ## Why are these changes needed?
   
   Follow-up to #2752, addressing the three **non-blocking** items @laskoviymishka raised in the approval review. The approved PR is left untouched; this stacks on top of it (the diff collapses to the single new commit once #2752 merges).
   
   Quoted from the approval review:
   
   1. > *The prefix buffer can hold a whole decoded file in memory when a file never changes or its first change is near EOF. You've already noted the size-capped fallback as future work in the code — I'd just surface that memory ceiling in the public `rewrite()` docs so consumers compacting multi-GB files know the bound up front.*
   
      The worst-case bound was only stated in an internal comment.  now carries a  rustdoc section: a file that never changes — or whose first change sits at its very end — buffers its entire decoded contents, one file at a time, which can be several GB for a compaction-sized file.
   
   2. > *Replacement partition metadata is trusted verbatim from the source file. A rewriter that mutates a partition-source column would silently record a partition that disagrees with its rows; a debug-assert (or making the commit adapter responsible for validation) would keep that a footgun rather than a corruption path.*
   
      Added a : in debug builds every emitted batch (including ones only buffered into the prefix) is checked against the source file's partition using the existing  /  infrastructure, and trips a  with a message naming the contract. Compiled out entirely in release builds, where per-row partition calculation would be too expensive for the rewrite hot path. A  regression test drives a rewriter that bumps an identity-partitioned column and asserts the guard fires; the existing partition-preservation end-to-end test exercises the guard's happy path.
   
   3. > *The sort-order and partition-spec choices are the same kind of divergence (replacements stay unsorted and keep the source spec, no repartition) and deserve a matching doc line.*
   
      The module docs now state explicitly that replacement files mirror their source file's layout — planned snapshot's schema (no promotion), source partition spec (no repartition), unsorted (no sort order applied) — as a deliberate divergence from Java's RewriteDataFiles / Spark DML, with an opt-in promote-to-current-schema rewrite noted as follow-up work.
   
   No public API changes ( unchanged); release behavior is byte-for-byte identical.
   
   ## Does this PR introduce any user-facing change?
   
   No. Rustdoc additions on the COW rewrite primitive plus a debug-build-only assertion.
   
   ## How was this patch tested?
   
   - `cargo fmt --all -- --check`
   - `cargo clippy -p iceberg --all-targets -- -D warnings`
   - `cargo test -p iceberg` — 1797 passed, 0 failed (plus doctests)
   - New: `cow_rewrite_partition_column_mutation_trips_debug_guard` (should-panic regression test for item 2)


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] fix(cow_rewrite): guard partition preservation and document rewrite() trade-offs [iceberg-rust]

Posted by "u70b3 (via GitHub)" <gi...@apache.org>.
u70b3 closed pull request #3296: fix(cow_rewrite): guard partition preservation and document rewrite() trade-offs
URL: https://github.com/apache/iceberg-rust/pull/3296


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org