feat: expose info for new ORC reader (FB-2919) - #41
Merged
Conversation
… start A caller that owns its own I/O needs to know which bytes a stripe read will touch before liborc asks for them, so it can fetch them up front and serve the reads from memory. Add ORCFileReader::GetStripeStreamRanges, which reports the data streams of every column liborc selects for a set of include_indices. The selected set is taken from RowReader::getSelectedColumns rather than recomputed, because selection expands beyond the requested indices to their descendants and all of their ancestors -- an ancestor's PRESENT stream carries the nullability of the struct holding a requested leaf -- and a second implementation of that rule could only drift from the one the row reader uses. Building the row reader costs no stripe I/O: it selects off the file footer. Index streams are excluded. They are only needed to seek inside a stripe or to evaluate a search argument, and NextStripeReader was requesting them even when it had neither reason to: it seeks unconditionally, and seekToRow loads the stripe index even for a zero-row seek, which drags in the bloom filters alongside the row indexes. The bloom filters can be an order of magnitude larger than the indexes and nothing reads them without a search argument. Skip the seek when already positioned at the stripe's first row; the row reader starts there regardless, and previousRow_ is only observable through getRowNumber(), which next() recomputes. Also relax CordedRandomAccessFile::ReadAt(position, nbytes, void*) from final to override. The ORC adapter reaches a file through liborc's InputStream::read, which copies into a caller-owned buffer and so can never take the corded path; leaving it final forces such a file to be a separate class. The Buffer-returning overload stays final.
GetStripeStreamRanges cannot answer until the stripe footer is readable, since the footer is what names the streams. A caller doing its own I/O therefore needs the footer's extent before it has the footer -- and StripeInformation, which it already has, does not carry it. Report it from the file footer, where the offset and the index, data and footer lengths all sit and are already parsed. This reads nothing, so it composes with GetStripeStreamRanges: ask for the range, make it available, then ask what the streams are.
tobias-fire
approved these changes
Aug 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Our new ORC reader needs access to some currently not exposed information. Expose it.
We also need to override
CordedRandomAccessFile::ReadAtfor ORC, which was so far declaredfinaland always returned NotImplemented.Lastly, stop seeking to the stripe start in
ORCFileReader::Impl::NextStripeReaderfor 0-row-seeks: this allows us to not fetch e.g. Bloom filter streams if we don't want them.