yihua commented on code in PR #19304:
URL: https://github.com/apache/hudi/pull/19304#discussion_r4041638751
##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java:
##########
@@ -413,6 +414,17 @@ public static final String getDefaultPayloadClassName() {
.sinceVersion("1.1.0")
.withDocumentation("This property when set, will define how two versions
of the record will be merged together when records are partially formed");
+ public static final ConfigProperty<String> COMPLEX_KEYGEN_ENCODING =
ConfigProperty
+ .key("hoodie.table.complex.keygen.encoding")
+ .noDefaultValue()
+ .sinceVersion("1.1.0")
Review Comment:
Should this be changed to `1.3.0`, or should `1.1.2` and `1.2.2` be added
for minor releases?
##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java:
##########
@@ -1387,6 +1399,19 @@ public Option<PartialUpdateMode> getPartialUpdateMode() {
}
}
+ /**
+ * @return the persisted record key encoding of a single-field complex key
generator table, if the
+ * table was upgraded from version 8 or below and the encoding was stamped.
Empty for tables created at
+ * version 9 and above (which always use {@link
ComplexKeyGenEncoding#FIELD_PREFIXED}) and for tables
+ * below version 9, where the property is never persisted.
+ */
+ public Option<ComplexKeyGenEncoding> getComplexKeyGenEncoding() {
+ if (getTableVersion().greaterThanOrEquals(HoodieTableVersion.NINE) &&
contains(COMPLEX_KEYGEN_ENCODING)) {
+ return
Option.of(ComplexKeyGenEncoding.fromString(getString(COMPLEX_KEYGEN_ENCODING)));
+ }
Review Comment:
Should we remove the condition
`getTableVersion().greaterThanOrEquals(HoodieTableVersion.NINE)` so that table
version 6 and 8 that are affected by the complex keygen issue can be
automatically mitigated by the new table config as well?
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/keygen/KeyGenUtils.java:
##########
@@ -436,15 +456,154 @@ public static String getComplexKeygenErrorMessage(String
operation) {
+ "`hoodie.write.complex.keygen.validation.enable=false` to skip this
validation.";
}
+ /**
+ * Whether a complex key generator with a single record key field prepends
the field name to the key.
+ *
+ * <p>The encoding persisted on the table ({@link
HoodieTableConfig#COMPLEX_KEYGEN_ENCODING}, stamped by the
+ * table version 8 to 9 upgrade from the table's existing data) is
authoritative whenever it is present in
+ * the properties, because it describes what the data actually carries.
Otherwise the write table version
+ * decides: 9 and above always prefix, 8 and below follow {@code
hoodie.write.complex.keygen.new.encoding}.
+ */
public static boolean
encodeSingleKeyFieldNameForComplexKeyGen(TypedProperties props) {
+ String tableEncoding =
props.getProperty(HoodieTableConfig.COMPLEX_KEYGEN_ENCODING.key());
+ if (!StringUtils.isNullOrEmpty(tableEncoding)) {
+ return
ComplexKeyGenEncoding.fromString(tableEncoding).encodesFieldName();
+ }
int tableVersionCode = ConfigUtils.getIntWithAltKeys(props,
WRITE_TABLE_VERSION);
HoodieTableVersion tableVersion =
HoodieTableVersion.fromVersionCode(tableVersionCode);
return tableVersion.greaterThanOrEquals(HoodieTableVersion.NINE)
|| !ConfigUtils.getBooleanWithAltKeys(props,
COMPLEX_KEYGEN_NEW_ENCODING);
}
+ /**
+ * Whether the record key encoding of a single-field complex key generator
table is unknown to readers,
+ * i.e. the table is below version 9, where the encoding is a per-write
decision that is never recorded.
+ */
public static boolean mayUseNewEncodingForComplexKeyGen(HoodieTableConfig
tableConfig) {
return tableConfig.getTableVersion().lesserThan(HoodieTableVersion.NINE)
&& isComplexKeyGeneratorWithSingleRecordKeyField(tableConfig);
}
+
+ /**
+ * The record key encoding a single-field complex key generator table is
known to carry.
+ *
+ * @return the persisted encoding, or {@link
ComplexKeyGenEncoding#FIELD_PREFIXED} for a table at version 9
+ * and above without the property (created there, never migrated); empty
when the table does not use a
+ * single-field complex key generator or is below version 9 (encoding not
recorded, see
+ * {@link #mayUseNewEncodingForComplexKeyGen}).
+ */
+ public static Option<ComplexKeyGenEncoding>
resolveComplexKeyGenEncoding(HoodieTableConfig tableConfig) {
+ if (!isComplexKeyGeneratorWithSingleRecordKeyField(tableConfig)
+ || tableConfig.getTableVersion().lesserThan(HoodieTableVersion.NINE)) {
+ return Option.empty();
+ }
+ Option<ComplexKeyGenEncoding> persisted =
tableConfig.getComplexKeyGenEncoding();
+ return Option.of(persisted.isPresent() ? persisted.get() :
ComplexKeyGenEncoding.FIELD_PREFIXED);
Review Comment:
It would be simpler to always follow the `ComplexKeyGenEncoding` in table
config for complex keygen with single record key for table version 6 and above;
if the table config does not exist in this case, it should always be inferred,
backfilled in the table config and continue. If it cannot be inferred due to
conflicting evidence, fail the write to let user chime in. So we can get rid of
default behavior.
--
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]
