wombatu-kun opened a new pull request, #19389: URL: https://github.com/apache/hudi/pull/19389
### Describe the issue this Pull Request addresses `HoodieFileGroupReader`'s constructor assigns the merged record-merge properties to its field, but the constructor parameter of the same name shadows that field for the rest of the body. So the schema handler, the record merger and the merge-type lookup get the caller's raw properties, while `initRecordIterators()` - which has no such parameter in scope - gives the record buffer the merged ones. `ConfigUtils.getMergeProps` is the only place that strips the `hoodie.record.merge.property.` prefix back to the plain `hoodie.payload.delete.field` / `.marker` keys that `DeleteContext` reads, and a table at version 9 or later persists a custom delete marker only in prefixed form. On such a table the schema handler's `DeleteContext` therefore resolves no marker: the delete column is left out of the required schema, and `FileGroupRecordBuffer`, which takes its `DeleteContext` from that same handler, can never let `RecordContext.isCustomDeleteRecord` fire. No read-path caller pre-merges the properties; every write-path caller does, which is why the write side is unaffected. `HoodieLsmFileGroupReader` repeats the identical shadowing. ### Summary and Changelog - `HoodieFileGroupReader` and `HoodieLsmFileGroupReader`: hold the merged properties in a local and use it for `initRecordMerger`, the merge-type lookup and both schema-handler constructions, so the parameter can no longer shadow them. - New `TestFileGroupReaderDeleteMarkerProps` builds a reader over a version-9 table config whose delete marker exists only in prefixed form, with reader properties carrying no plain delete keys, and asserts the installed schema handler resolves the marker and projects the delete column. This is the regression test: it fails on the current code and passes with the fix. - `TestHoodieFileGroupReaderOnSpark.testCustomDelete` gains two parameters that read the table without restating the delete options, which is what a query that just loads the path looks like. See below on what this does and does not prove. ### Impact Only tables that actually have record-merge properties are affected: `getMergeProps` returns the caller's properties object unchanged when the table has none. On the affected tables the reader now resolves the custom delete marker and projects the delete column. The two new Spark parameters pass both with and without the fix, and I am adding them as coverage rather than as proof. A Spark write-then-read round trip cannot surface this defect, because the Spark write path flags marker-carrying rows as deletes at record creation (`HoodieCreateRecordUtils`), so the reader never needs the marker for them. What they do pin is that such a query returns the right rows without the delete options being restated, which nothing covered before. The read-side exposure is for records that carry the marker without having been flagged at write time. ### Risk Level low The constructor now passes on the caller's properties overlaid with the table's own record-merge properties - exactly what the record buffer in the same class already received. Only three keys are ever written under that prefix (delete field, delete marker, partial-update unavailable value), and no `HoodieRecordMerger` here overrides `getMandatoryFieldsForMerging` or reads `properties`. Verified by the new test plus the existing schema-handler, record-buffer, `ConfigUtils` and Spark file-group-reader suites. ### 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]
