sudojha commented on PR #2345: URL: https://github.com/apache/iceberg-go/pull/2345#issuecomment-6506535151
Thanks for the thorough review @zeroshade ! Addressed in 5fa302d: 1. **Transformed fields:** the merge is now taken only when every field of the order is identity (`presortedPartitions`). Other claimable orders, e.g. `[day(ts)]`, go through the in-memory sort and still record the id. Added a test with a claimable `[id, truncate(p)]` order. 2. **Fan-in:** a partition with more files than the scan concurrency (`WithCompactionScanConcurrency`, `GOMAXPROCS` by default) now falls back to the in-memory path, so open inputs are bounded the same way as bin-pack. Docs now say up to about five batches per merged file. Lifting the cap with a multi-pass merge fits Phase 2 (spill). 3. **Interleaved inputs:** the merge now records runs as (batch, row range) and builds each output batch with the existing `Take` gather; an output batch that is a single run stays a zero-copy slice. On 2 × 131k rows × 32 int64: interleaved 1.32 s → 64 ms, disjoint unchanged at 3 ms. Added `BenchmarkMergeInterleaved` / `BenchmarkMergeDisjoint`. Nits: `api.md` now says `sort_order_id` is recorded only when the order is claimable, and the in-memory path notes the ~2× decoded size and that a table's first sort rewrite always takes it. Re apache/arrow-go#1430: thanks! I've kept the row comparator for now. Once a release includes `FloatTotalOrder`, I'll follow up to use the kernel on the in-memory path, keeping the comparator for the merge since Arrow's row comparator isn't exported. The two orderings stay compatible: totalOrder additionally orders NaN payloads, which our comparator treats as equal, so kernel output still passes the merge's sortedness check. -- 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]
