JanKaul commented on PR #2961:
URL: https://github.com/apache/iceberg-rust/pull/2961#issuecomment-5451146813

   Did an independent verification pass on 06edf3c26 from a clean checkout: 
cargo test -p iceberg --lib → 1576 passed / 0 failed, arrow:: subset 169 
passed, and clippy/fmt --check both clean. So CI green reproduces locally.
   
   I also reviewed the hash-probe for silent mis-deletion risks and it holds up:
   - Datum hashes/compares over both type and value, and layout_matches checks 
field-id and type — so the per-row promotion (needs_promotion → 
datum.to(target)) is exactly what keeps probe keys comparable with the parsed 
delete keys under schema evolution. Nicely covered by the decimal-widening and 
int32→int64 tests.
   - Null-matches-null equality is correct and tested.
   - The multi-set AND-ing (keep = !set.contains; RowFilter ANDs) correctly 
deletes a row that matches any layout set, and the probe key is fully rewritten 
each row with a safe per-cell take().
   - Pipeline-wise, scan predicate + eq-delete predicates land in the RowFilter 
while positional deletes still drive the RowSelection, and they compose 
correctly.
   
   The test suite now covers essentially the whole gap list from the first 
review (scan-pred × eq-delete intersection, pos+eq+pred, differing physical 
types through the real load path, decimal promotion, float/double 
canonicalization, empty set, absent key column). The absent-required-column → 
initial_default divergence is clearly documented and test-pinned; agree it's a 
good follow-up rather than a blocker here.
   
   LGTM from my (non-committer) review; the remaining threads read as agreed 
follow-ups. Would be great to get a committer's eyes on it to move it forward.


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