cshuo commented on code in PR #19415:
URL: https://github.com/apache/hudi/pull/19415#discussion_r3687877296
##########
hudi-common/src/test/java/org/apache/hudi/common/table/read/buffer/TestKeyBasedFileGroupRecordBuffer.java:
##########
@@ -74,6 +79,27 @@ class TestKeyBasedFileGroupRecordBuffer extends
BaseTestFileGroupRecordBuffer {
private final IndexedRecord testRecord6DeleteByCustomMarker =
createTestRecord("6", 3, 2L);
private final IndexedRecord testRecord7 = createTestRecord("7", 1, 5L);
+ /**
+ * Every buffer in this hierarchy must funnel records into the buffer
through the same
+ * merge-then-put path and expose the same partial-merge state, so these
methods are sealed with
+ * {@code final} and subclasses such as {@code
PositionBasedFileGroupRecordBuffer} and
+ * {@code SortedKeyBasedFileGroupRecordBuffer} cannot replace that behavior.
The methods that a
+ * subclass legitimately specializes (block processing, base-record
advancement, buffer type) are
+ * deliberately left open. This test fails if a {@code final} modifier is
dropped.
+ */
+ @Test
Review Comment:
This test does not enforce the stated invariant that every buffer funnels
records through the same merge-then-put path.
`PositionBasedFileGroupRecordBuffer` still writes directly to `records` in
`fallbackToKeyBasedBuffer` and the COMMIT_TIME_ORDERING delete path, while
subclasses retain protected access to the map and can override block
processing. The test only verifies that these two methods remain final. Please
narrow the test documentation/name/message to that exact guarantee, or
centralize the mutations behind a non-overridable helper if the stronger funnel
invariant is intended.
--
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]
