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: [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]
