cshuo commented on code in PR #19804:
URL: https://github.com/apache/hudi/pull/19804#discussion_r3909984950
##########
hudi-common/src/main/java/org/apache/hudi/common/table/log/AbstractHoodieLogRecordScanner.java:
##########
@@ -220,9 +230,44 @@ protected AbstractHoodieLogRecordScanner(HoodieStorage
storage, String basePath,
}
this.partitionNameOverrideOpt = partitionNameOverride;
+ this.deletePartitionPathOpt = resolveDeletePartitionPath(basePath,
logFilePaths, partitionNameOverride);
this.recordType = recordMerger.getRecordType();
}
+ private static Option<String> resolveDeletePartitionPath(String basePath,
List<String> logFilePaths,
+ Option<String>
partitionNameOverride) {
+ if (partitionNameOverride.isPresent()) {
+ return partitionNameOverride;
+ }
+ if (logFilePaths.isEmpty()) {
+ return Option.empty();
+ }
+ return Option.of(getRelativePartitionPath(
+ new StoragePath(basePath), new
StoragePath(logFilePaths.get(0)).getParent()));
+ }
+
+ private HoodieReaderContext<IndexedRecord> getOrCreateDeleteReaderContext() {
+ if (deleteReaderContext == null) {
+ HoodieTableConfig tableConfig = hoodieTableMetaClient.getTableConfig();
+ TypedProperties mergeProps = ConfigUtils.getMergeProps(payloadProps,
tableConfig);
+ HoodieReaderContext<IndexedRecord> readerContext = new
HoodieAvroReaderContext(
Review Comment:
[P2] Inject the reader context through the scanner builder
The scanner should not construct a `HoodieAvroReaderContext` internally or
derive its reader configuration from the scanner's payload-only properties. The
external caller owns the engine and complete runtime configuration, so it is
the appropriate place to create the reader context.
Please follow the `HoodieFileGroupReader` construction pattern: add
`Builder#withReaderContext(...)`, have callers such as Hadoop MR realtime
readers, the Spark procedure, and tests construct the appropriate context, and
pass it into the scanner.
##########
hudi-common/src/main/java/org/apache/hudi/common/table/log/AbstractHoodieLogRecordScanner.java:
##########
@@ -220,9 +230,44 @@ protected AbstractHoodieLogRecordScanner(HoodieStorage
storage, String basePath,
}
this.partitionNameOverrideOpt = partitionNameOverride;
+ this.deletePartitionPathOpt = resolveDeletePartitionPath(basePath,
logFilePaths, partitionNameOverride);
this.recordType = recordMerger.getRecordType();
}
+ private static Option<String> resolveDeletePartitionPath(String basePath,
List<String> logFilePaths,
+ Option<String>
partitionNameOverride) {
+ if (partitionNameOverride.isPresent()) {
+ return partitionNameOverride;
+ }
+ if (logFilePaths.isEmpty()) {
+ return Option.empty();
+ }
+ return Option.of(getRelativePartitionPath(
+ new StoragePath(basePath), new
StoragePath(logFilePaths.get(0)).getParent()));
+ }
+
+ private HoodieReaderContext<IndexedRecord> getOrCreateDeleteReaderContext() {
+ if (deleteReaderContext == null) {
+ HoodieTableConfig tableConfig = hoodieTableMetaClient.getTableConfig();
+ TypedProperties mergeProps = ConfigUtils.getMergeProps(payloadProps,
tableConfig);
+ HoodieReaderContext<IndexedRecord> readerContext = new
HoodieAvroReaderContext(
+ hoodieTableMetaClient.getStorageConf(), tableConfig, instantRange,
Option.empty(), mergeProps);
+ readerContext.setHasLogFiles(true);
+ readerContext.setHasBootstrapBaseFile(false);
+ readerContext.setShouldMergeUseRecordPosition(false);
+
readerContext.setTablePath(hoodieTableMetaClient.getBasePath().toString());
+ readerContext.setLatestCommitTime(latestInstantTime);
+
readerContext.getRecordContext().setPartitionPath(deletePartitionPathOpt.orElse(null));
+ readerContext.initRecordMerger(mergeProps);
Review Comment:
[P2] Avoid fully initializing the delete-only reader context
The scanner only uses this reader context to materialize delete records.
Native delete blocks use it to read records with the explicitly supplied
`deleteLogSchema` and extract the record key and ordering value; legacy delete
blocks only use its `RecordContext` to normalize the ordering value. Neither
path uses a record merger or requires a `FileGroupReaderSchemaHandler`.
Please remove `initRecordMerger(...)` and `setSchemaHandler(...)` from this
path. For native log files, `HoodieAvroReaderContext#getFileRecordIterator`
should check `isLogFile` before accessing the schema handler, since the
log-file branch already uses the explicitly supplied required schema.
--
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]
