rangareddy opened a new pull request, #19415:
URL: https://github.com/apache/hudi/pull/19415
### Describe the issue this Pull Request addresses
Closes #16920 (HUDI-9200).
`KeyBasedFileGroupRecordBuffer` is subclassed by
`PositionBasedFileGroupRecordBuffer` and
`SortedKeyBasedFileGroupRecordBuffer`. Both legitimately specialize how log
blocks are consumed and
how the base-file iterator is advanced, but the step that actually mutates
the buffer — merge the
incoming record against what is already buffered, then store the result —
must behave identically for
every buffer in the hierarchy. Today nothing expresses that, so a subclass
can silently replace it.
### Summary and Changelog
Marks the two methods that no subclass overrides as `final`, so the shared
merge path cannot be
replaced by a subclass, and documents why:
- `KeyBasedFileGroupRecordBuffer#processNextDataRecord` → `final`.
Subclasses still choose the
identifier a record is buffered under (a record key here, a record
position in
`PositionBasedFileGroupRecordBuffer`), which is the parameter, not the
behavior.
- `KeyBasedFileGroupRecordBuffer#isPartialMergingEnabled` → `final`. The
`enablePartialMerging` flag is
owned and toggled by the base buffer while processing data blocks.
- Adds
`TestKeyBasedFileGroupRecordBuffer#keyMergeBehaviorIsSealedAgainstSubclasses`,
which asserts
both modifiers via reflection. `final` is compiler-enforced, but nothing
otherwise stops the keyword
from being dropped later; the test was confirmed to fail for each method
independently before the
change.
The remaining methods (`getBufferType`, `processDataBlock`,
`processDeleteBlock`, `containsLogRecord`,
`hasNextBaseRecord`, `doHasNext`) are **deliberately left open** — each is
overridden by
`PositionBasedFileGroupRecordBuffer` and/or
`SortedKeyBasedFileGroupRecordBuffer` today.
Note on scope: the ticket is phrased as preventing
`PositionBasedFileGroupRecordBuffer` from overriding
key functions, and `PositionBased` does not override either method sealed
here. If the intent was
instead to refactor `PositionBased` so it stops overriding the
block-processing methods (each of which
currently opens with a `if (!getShouldMergeUseRecordPosition()) { super.…;
return; }` fallback) and then
seal those, that is a larger design change and I'm happy to defer to the
ticket's assignee on it. This
PR is the behavior-preserving subset.
### Impact
None for users. `final` is enforced at compile time and these are internal
reader classes — nothing in
`org.apache.hudi.common.table.read.buffer` is annotated `@PublicAPIClass`.
No public API, config, or
on-disk format change. Making a public method `final` is binary-compatible;
it would only affect an
out-of-tree subclass that overrides these methods, and no in-tree subclass,
test, or mock does.
No performance claim is made: these call sites are already monomorphic and
devirtualized by the JIT.
### Risk Level
none
### Documentation Update
none — no new config, no default value change, no user-facing behavior
change.
### 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
- [x] CI passes on my PR
--
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]
