yihua commented on code in PR #13615:
URL: https://github.com/apache/hudi/pull/13615#discussion_r2277390249
##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/utils/HoodieWriterClientTestHarness.java:
##########
@@ -554,7 +554,7 @@ protected List<HoodieRecord<RawTripTestPayload>>
dedupForCopyOnWriteStorage(Hood
.getReaderContextFactoryForWrite(metaClient,
HoodieRecord.HoodieRecordType.AVRO, writeConfig.getProps()).getContext();
List<String> orderingFieldNames = getOrderingFieldNames(
readerContext.getMergeMode(), writeClient.getConfig().getProps(),
metaClient);
- RecordMergeMode recordMergeMode =
HoodieTableConfig.inferCorrectMergingBehavior(null,
writeConfig.getPayloadClass(), null,
+ RecordMergeMode recordMergeMode =
HoodieTableConfig.inferMergingConfigs(metaClient.getTableConfig().getRecordMergeMode(),
writeConfig.getPayloadClass(), null,
Review Comment:
Should `metaClient.getTableConfig().getRecordMergeMode()` directly be used
instead of calling `inferMergingConfigs`? Trying to understand the intention
here in test.
##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java:
##########
@@ -230,6 +265,12 @@ public class HoodieTableConfig extends HoodieConfig {
.withDocumentation("Payload class to use for performing merges,
compactions, i.e merge delta logs with current base file and then "
+ " produce a new base file.");
+ public static final ConfigProperty<String> LEGACY_PAYLOAD_CLASS_NAME =
ConfigProperty
+ .key("hoodie.legacy.payload.class")
+ .noDefaultValue()
+ .deprecatedAfter("1.1.0")
Review Comment:
```suggestion
.sinceVersion("1.1.0")
```
##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java:
##########
@@ -121,10 +132,34 @@ public class HoodieTableConfig extends HoodieConfig {
public static final String PARTIAL_UPDATE_CUSTOM_MARKER =
"hoodie.write.partial.update.custom.marker";
public static final String DEBEZIUM_UNAVAILABLE_VALUE =
"__debezium_unavailable_value";
// This prefix is used to set merging related properties.
- // A reader might need to read some merger properties to function as
expected,
- // and Hudi stores properties with this prefix so the reader parses these
properties,
- // and produces a map of key value pairs (Key1->Value1, Key2->Value2, ...)
to use.
- public static final String MERGE_PROPERTIES_PREFIX =
"hoodie.table.merge.properties.";
+ // A reader might need to read some writer properties to function as
expected,
+ // and Hudi stores properties with this prefix so the reader parses these
properties to fetch any custom property.
+ public static final String MERGE_CUSTOM_PROPERTY_PREFIX =
"hoodie.merge.custom.property.";
+ public static final Set<String> PAYLOADS_UNDER_DEPRECATION =
Collections.unmodifiableSet(
+ new HashSet<>(Arrays.asList(
+ AWSDmsAvroPayload.class.getName(),
Review Comment:
Do we need to remove `BootstrapRecordPayload` as well?
##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java:
##########
@@ -121,10 +132,34 @@ public class HoodieTableConfig extends HoodieConfig {
public static final String PARTIAL_UPDATE_CUSTOM_MARKER =
"hoodie.write.partial.update.custom.marker";
public static final String DEBEZIUM_UNAVAILABLE_VALUE =
"__debezium_unavailable_value";
// This prefix is used to set merging related properties.
- // A reader might need to read some merger properties to function as
expected,
- // and Hudi stores properties with this prefix so the reader parses these
properties,
- // and produces a map of key value pairs (Key1->Value1, Key2->Value2, ...)
to use.
- public static final String MERGE_PROPERTIES_PREFIX =
"hoodie.table.merge.properties.";
+ // A reader might need to read some writer properties to function as
expected,
+ // and Hudi stores properties with this prefix so the reader parses these
properties to fetch any custom property.
+ public static final String MERGE_CUSTOM_PROPERTY_PREFIX =
"hoodie.merge.custom.property.";
+ public static final Set<String> PAYLOADS_UNDER_DEPRECATION =
Collections.unmodifiableSet(
+ new HashSet<>(Arrays.asList(
+ AWSDmsAvroPayload.class.getName(),
+ DefaultHoodieRecordPayload.class.getName(),
+ EventTimeAvroPayload.class.getName(),
+ MySqlDebeziumAvroPayload.class.getName(),
+ OverwriteNonDefaultsWithLatestAvroPayload.class.getName(),
+ OverwriteWithLatestAvroPayload.class.getName(),
+ PartialUpdateAvroPayload.class.getName(),
+ PostgresDebeziumAvroPayload.class.getName())));
+
+ public static final Set<String> EVENT_TIME_BASED_PAYLOADS =
Collections.unmodifiableSet(
Review Comment:
```suggestion
public static final Set<String> EVENT_TIME_ORDERING_PAYLOADS =
Collections.unmodifiableSet(
```
##########
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieRecordPayload.java:
##########
@@ -182,16 +184,32 @@ static String getPayloadClassName(HoodieConfig config) {
}
static String getPayloadClassName(Properties props) {
- return
getPayloadClassNameIfPresent(props).orElse(HoodieTableConfig.DEFAULT_PAYLOAD_CLASS_NAME);
+ Option<String> payloadOpt = getPayloadClassNameIfPresent(props);
+ if (payloadOpt.isPresent()) {
+ return payloadOpt.get();
+ }
+ // Note: starting from version 9, payload class is not necessary set, but
+ // merge mode must exist. Therefore, we use merge mode to infer
+ // the payload class for certain corner cases, like for MIT command.
+ if (props.containsKey(RECORD_MERGE_MODE.key())
Review Comment:
nit: use `ConfigUtils`
##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java:
##########
@@ -801,19 +842,125 @@ public String getPayloadClass() {
return HoodieRecordPayload.getPayloadClassName(this);
}
+ public String getLegacyPayloadClass() {
+ return getStringOrDefault(LEGACY_PAYLOAD_CLASS_NAME, "");
+ }
+
public String getRecordMergeStrategyId() {
return getString(RECORD_MERGE_STRATEGY_ID);
}
+ /**
+ * Handle table config creation logic when creating a table for Table
Version 9,
+ * which is based on the logic of table version < 9, and then tuned for
version 9 logic.
+ * This approach fits the same behavior of upgrade from 8 to 9.
+ */
+ public static Map<String, String>
inferCorrectMergingBehaviorV9TblCreation(RecordMergeMode recordMergeMode,
Review Comment:
```suggestion
public static Map<String, String>
inferMergingConfigsForV9TableCreation(RecordMergeMode recordMergeMode,
```
--
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]
