etseidl commented on PR #10149: URL: https://github.com/apache/arrow-rs/pull/10149#issuecomment-4731867222
Thanks for the review @alamb! > I think we could potentially reduce the change required in this PR (and not have to change any APIs) by simply not populating the ordinal field for row groups with indexes that are too large and keep returning i16 > > What do you think? Given that our use of ordinal is an implementation detail, I think it's fine to promote it to `i32` internally. This allows continued use of the row numbering and bloom filters in the > 32k row groups case with minimal changes. Otherwise, we'd be adding additional checks for feature enablement in the parsing code, or just waiting for things to error out later. For me, the issue is just do we wait until 60.0 for this change, or keep the public API unchanged and internally use 32-bit versions of API calls? -- 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]
