yihua commented on code in PR #11634:
URL: https://github.com/apache/hudi/pull/11634#discussion_r1742778508
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java:
##########
@@ -1396,27 +1395,25 @@ protected void preWrite(String instantTime) {
* records and hence is more suited to bulkInsert for write performance.
*
* @param instantTime - Action instant time for this commit
- * @param partitionType - The MDT partition to which records are to be
committed
+ * @param partitionName - The MDT partition to which records are to be
committed
* @param records - records to be bulk inserted
* @param fileGroupCount - The maximum number of file groups to which the
records will be written.
*/
protected abstract void bulkCommit(
- String instantTime, MetadataPartitionType partitionType,
HoodieData<HoodieRecord> records,
+ String instantTime, String partitionName, HoodieData<HoodieRecord>
records,
Review Comment:
Is this an issue we need to fix? Do we have a JIRA to fix it?
##########
hudi-client/hudi-java-client/src/main/java/org/apache/hudi/metadata/JavaHoodieBackedTableMetadataWriter.java:
##########
@@ -87,7 +87,7 @@ protected void initRegistry() {
}
@Override
- protected void commit(String instantTime, Map<MetadataPartitionType,
HoodieData<HoodieRecord>> partitionRecordsMap) {
+ protected void commit(String instantTime, Map<String,
HoodieData<HoodieRecord>> partitionRecordsMap) {
Review Comment:
I still have this question: could we still keep `MetadataPartitionType` in
the map and all other places?
Whenever needed, call `metadataPartitionType.getPartitionPath` and use the
value. Does this map keep records per partition or per index type, because
functional or secondary indexes may contain multiple index definitions? Even
so, this PR does not seem to fix that mapping.
##########
hudi-client/hudi-flink-client/src/main/java/org/apache/hudi/metadata/FlinkHoodieBackedTableMetadataWriter.java:
##########
@@ -109,12 +109,13 @@ protected List<HoodieRecord>
convertHoodieDataToEngineSpecificData(HoodieData<Ho
}
@Override
- protected void bulkCommit(String instantTime, MetadataPartitionType
partitionType, HoodieData<HoodieRecord> records, int fileGroupCount) {
- commitInternal(instantTime, Collections.singletonMap(partitionType,
records), true, Option.empty());
+ protected void bulkCommit(String instantTime, String partitionName,
HoodieData<HoodieRecord> records, int fileGroupCount) {
+ // TODO: functional and secondary index are not supported with Flink yet,
but we should fix the partition name when we support them.
Review Comment:
Using a strong type `MetadataPartitionType` is better than `String` so
developer knows what's expected here.
--
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]
