nsivabalan commented on code in PR #13495:
URL: https://github.com/apache/hudi/pull/13495#discussion_r2220237385
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -3460,7 +3460,7 @@ public Builder withConcatHandleClassName(String
className) {
}
public Builder withFileGroupReaderMergeHandleClassName(String className) {
- writeConfig.setValue(FILE_GROUP_READER_MERGE_HANDLE_CLASS_NAME,
className);
+ writeConfig.setValue(COMPACT_MERGE_HANDLE_CLASS_NAME, className);
Review Comment:
same here
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/table/action/commit/BaseSparkCommitActionExecutor.java:
##########
@@ -382,39 +383,21 @@ public Iterator<List<WriteStatus>> handleUpdate(String
partitionPath, String fil
// these are updates
HoodieMergeHandle mergeHandle = getUpdateHandle(partitionPath, fileId,
recordItr);
- return handleUpdateInternal(mergeHandle, fileId);
- }
-
- protected Iterator<List<WriteStatus>>
handleUpdateInternal(HoodieMergeHandle<?, ?, ?, ?> mergeHandle, String fileId)
- throws IOException {
- if (mergeHandle.getOldFilePath() == null) {
- throw new HoodieUpsertException(
- "Error in finding the old file path at commit " + instantTime + "
for fileId: " + fileId);
- } else {
- if (mergeHandle.baseFileForMerge().getBootstrapBaseFile().isPresent()) {
- Option<String[]> partitionFields =
table.getMetaClient().getTableConfig().getPartitionFields();
- Object[] partitionValues =
SparkPartitionUtils.getPartitionFieldVals(partitionFields,
mergeHandle.getPartitionPath(),
-
table.getMetaClient().getTableConfig().getBootstrapBasePath().get(),
- mergeHandle.getWriterSchema(), (Configuration)
table.getStorageConf().unwrap());
- mergeHandle.setPartitionFields(partitionFields);
- mergeHandle.setPartitionValues(partitionValues);
- }
-
- mergeHandle.doMerge();
- }
-
- // TODO(vc): This needs to be revisited
- if (mergeHandle.getPartitionPath() == null) {
- LOG.info("Upsert Handle has partition path as null " +
mergeHandle.getOldFilePath() + ", "
- + mergeHandle.getWriteStatuses());
- }
-
- return
Collections.singletonList(mergeHandle.getWriteStatuses()).iterator();
+ return IOUtils.runMerge(mergeHandle, instantTime, fileId);
}
protected HoodieMergeHandle getUpdateHandle(String partitionPath, String
fileId, Iterator<HoodieRecord<T>> recordItr) {
- return HoodieMergeHandleFactory.create(operationType, config, instantTime,
table, recordItr, partitionPath, fileId,
+ HoodieMergeHandle mergeHandle =
HoodieMergeHandleFactory.create(operationType, config, instantTime, table,
recordItr, partitionPath, fileId,
taskContextSupplier, keyGeneratorOpt);
+ if (mergeHandle.getOldFilePath() != null &&
mergeHandle.baseFileForMerge().getBootstrapBaseFile().isPresent()) {
+ Option<String[]> partitionFields =
table.getMetaClient().getTableConfig().getPartitionFields();
Review Comment:
not sure why we don't have this for other engines?
is it that we don't have bootstrap support added to other engines.
even if it is so, we can just move these to common methods right. just that
it will be no op for other engines.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -1544,7 +1544,7 @@ public String getConcatHandleClassName() {
}
public String getFileGroupReaderMergeHandleClassName() {
- return getStringOrDefault(FILE_GROUP_READER_MERGE_HANDLE_CLASS_NAME);
+ return getStringOrDefault(COMPACT_MERGE_HANDLE_CLASS_NAME);
Review Comment:
we should always match the config name and getter methods.
`getCompactMergeHandleClassName`
--
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]
