zhuqi-lucas commented on PR #10449: URL: https://github.com/apache/arrow-rs/pull/10449#issuecomment-5174491169
Thanks @etseidl — resolved in c29948185 (happy to have done it myself, it was a useful conflict to think through 😄). The interesting part wasn't the i32 widening but the semantic overlap: #10149's `OrdinalAssigner` errors on mixed ordinal presence, while tolerating exactly that case is this PR's purpose. The merge keeps the post-decode `ensure_row_group_ordinals` as the single normalization point with the union of both behaviors: - **all present** → honor as written - **none present** (including the >32k-RG files from #10149, whose writers omit ordinals) → backfill from position, as `i32` - **mixed** → leave untouched; `RowNumberReader::try_new` still fails deterministically for consumers that need complete ordinals, and plain reads succeed The streaming assigner is superseded because mixed-tolerance needs the whole row-group list before deciding — a partial backfill that stops mid-file would make row numbers depend on which row groups a query selects. Verified locally: ordinal / row_number / writer / arrow_reader / metadata test suites all pass (261 tests). PTAL when you get a chance! -- 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]
