yihua opened a new pull request, #20001:
URL: https://github.com/apache/hudi/pull/20001

   ### Describe the issue this Pull Request addresses
   
   closes #19999
   
   `HoodieRecordReader` exposes the log records of a file group only as a 
stream, so a caller that needs to look one up by key or by position has to 
drain the iterator and rebuild an index that, in the buffered case, the record 
buffer is already holding. `HoodieMergedLogRecordReader` has had both accessors 
over the same buffer for a while, so code moving to the file group readers 
loses random access.
   
   ### Summary and Changelog
   
   Adds `getLogRecordsMap()` to the `HoodieRecordReader` interface, alongside 
the existing `getLogRecordsOnly()`, so the callers that already program to that 
interface can use it rather than downcasting to one concrete reader.
   
   - `HoodieFileGroupReader` returns a read-only view of the record buffer's 
own map, keyed by record key or by record position depending on the merge mode, 
at no extra cost.
   - `HoodieLsmFileGroupReader` merges as it streams and so has no index to 
hand back. It materializes one keyed by record key, and says so in its javadoc, 
so callers can stay on the iterator when a single pass is enough.
   - `UnmergedFileGroupRecordBuffer` (reading with 
`hoodie.datasource.merge.type=skip_merge`) indexes nothing, so it now refuses 
the map accessor the same way it already refuses `getLogRecordIterator()`. 
Inheriting the base implementation would have reported an empty map, which is a 
wrong answer rather than an absent one.
   - `ReusableKeyBasedRecordBuffer` builds no map of its own and so inherited 
`null` from `getLogRecords()`, and an NPE from `size()`. It now serves the 
records it was handed.
   - `HoodieFileGroupReader#getLogRecordsOnly()` threw a `NullPointerException` 
for a file group with no log records to merge, since the record buffer is only 
built when there is something to merge into the base file. It now returns an 
empty iterator, and the new accessor an empty map.
   
   Tests cover the keying in both merge modes, exclusion of base-file-only 
keys, the empty and skip-merge cases, the read-only view, that a merged read is 
unaffected by having read the map, the LSM implementation, and the reusable 
buffer's records and size. `HoodieFileGroupReaderTestHarness` now exposes the 
reader itself through `getFileGroupReader(...)` and `writeFileSlice(...)`, with 
the existing `getFileGroupIterator(...)` overloads delegating to them.
   
   Not addressed here, since it predates this change and a safe fix is not 
local to it: `initRecordIterators()` rebuilds the record buffer on every 
accessor call and leaves the previous one open (#20000).
   
   ### Impact
   
   New read method on the `HoodieRecordReader` interface, which is a 
source-incompatible change for any implementation outside this repo. Both 
in-tree implementations are updated. Behavior changes to existing code are 
limited to three cases that previously threw or returned `null`: 
`getLogRecordsOnly()` on a file group with nothing to merge, and 
`getLogRecords()` / `size()` on the reusable buffer.
   
   ### Risk Level
   
   low
   
   ### Documentation Update
   
   none
   
   ### Contributor's checklist
   
   - [x] Read through [contributor's 
guide](https://hudi.apache.org/contribute/how-to-contribute)
   - [x] Enough context is provided in the sections above
   - [x] Adequate tests were added if applicable
   


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

Reply via email to