yihua commented on code in PR #19304:
URL: https://github.com/apache/hudi/pull/19304#discussion_r4103155955
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/keygen/KeyGenUtils.java:
##########
@@ -436,15 +477,261 @@ 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>{@link HoodieTableConfig#COMPLEX_KEYGEN_ENCODING} wins whenever it is
present in the properties, because
+ * it describes what the table's data 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 = ConfigUtils.getStringWithAltKeys(props,
HoodieTableConfig.COMPLEX_KEYGEN_ENCODING, StringUtils.EMPTY_STRING);
+ 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);
Review Comment:
The fallback is still reachable for the shapes the property is never
recorded for: with `populateMetaFields=false` the table carries no encoding
(asserted in `TestHoodieTableMetaClient`), yet `ComplexAvroKeyGenerator` still
calls this to decide, and Kafka Connect has no backfill at all.
`COMPLEX_KEYGEN_NEW_ENCODING` is also read by
`resolveComplexKeyGenEncodingForWrite` when the encoding cannot be deduced and
validation is off, so dropping it here would not retire the config; narrowing
it to the untracked shapes and saying so in the javadoc seems the better move.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/keygen/KeyGenUtils.java:
##########
@@ -436,15 +477,261 @@ 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>{@link HoodieTableConfig#COMPLEX_KEYGEN_ENCODING} wins whenever it is
present in the properties, because
+ * it describes what the table's data 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 = ConfigUtils.getStringWithAltKeys(props,
HoodieTableConfig.COMPLEX_KEYGEN_ENCODING, StringUtils.EMPTY_STRING);
+ 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);
}
- public static boolean mayUseNewEncodingForComplexKeyGen(HoodieTableConfig
tableConfig) {
- return tableConfig.getTableVersion().lesserThan(HoodieTableVersion.NINE)
- && isComplexKeyGeneratorWithSingleRecordKeyField(tableConfig);
+ /**
+ * Whether the table's record key encoding is tracked by {@link
HoodieTableConfig#COMPLEX_KEYGEN_ENCODING}:
+ * a complex key generator with a single record key field and a populated
{@code _hoodie_record_key}.
+ * Without the meta field there is no stored key whose encoding could
diverge from the key generator's.
+ */
+ public static boolean requireComplexKeyGenEncodingTracked(HoodieTableConfig
tableConfig) {
+ return tableConfig.isComplexKeyGenWithSingleRecordKeyField() &&
tableConfig.isRecordKeyPopulated();
+ }
+
+ /**
+ * Cheap pre-check on the write config alone, before any table config is
loaded: whether the write may target a
+ * table whose encoding is tracked and not yet recorded. A write config that
already carries the recorded
+ * encoding (Spark merges the table config into the write options), names a
non-complex built-in key generator,
+ * keys on several fields or does not populate the meta fields cannot need
the recording, so callers skip
+ * loading the table config for it. A custom key generator class (such as
the wrapper Spark SQL configures) is
+ * decided by the table's own key generator settings when they are present,
and cannot be ruled out otherwise.
+ */
+ public static boolean mayNeedComplexKeyGenEncodingRecorded(HoodieConfig
writeConfig) {
+ if (writeConfig.contains(HoodieTableConfig.COMPLEX_KEYGEN_ENCODING)
Review Comment:
Agreed, and it is a table property only: `COMPLEX_KEYGEN_ENCODING` is
declared solely in `HoodieTableConfig` and appears in neither
`DataSourceWriteOptions` nor `FlinkOptions`, so there is no user-facing write
option for it. The `contains` check here is really asking whether the table
config has already been merged into the options, so the parameter name is what
misleads; renaming it and stating that in the javadoc would fix it.
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieSparkSqlWriter.scala:
##########
@@ -321,6 +321,7 @@ class HoodieSparkSqlWriterInternal {
.setPopulateMetaFields(populateMetaFields)
.setMetaFieldsModeFromString(metaFieldsMode)
.setRecordKeyFields(hoodieConfig.getString(RECORDKEY_FIELD))
+
.setComplexKeyGenEncoding(explicitComplexKeyGenEncoding(hoodieConfig))
Review Comment:
The builder already gates it: `HoodieTableMetaClient` records the property
only for a single-field complex key generator with a populated record key, so
passing it unconditionally writes nothing for other shapes. The part worth
tightening is that an encoding declared on an untracked shape is dropped
silently, which now has explicit coverage. The other `newTableBuilder()`
callers need no setter, since the builder defaults to FIELD_PREFIXED for the
tracked shape.
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieSparkSqlWriter.scala:
##########
@@ -780,6 +783,7 @@ class HoodieSparkSqlWriterInternal {
.setTableType(HoodieTableType.valueOf(tableType))
.setTableName(tableName)
.setRecordKeyFields(recordKeyFields)
+
.setComplexKeyGenEncoding(explicitComplexKeyGenEncoding(hoodieConfig))
Review Comment:
Same answer as above: the builder gates what it records, so this is inert
for untracked shapes.
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/streamer/HoodieStreamerUtils.java:
##########
@@ -116,7 +116,8 @@ public static Option<JavaRDD<HoodieRecord>>
createHoodieRecords(HoodieStreamer.C
props.setProperty(KeyGenUtils.RECORD_KEY_GEN_PARTITION_ID_CONFIG,
String.valueOf(TaskContext.getPartitionId()));
props.setProperty(KeyGenUtils.RECORD_KEY_GEN_INSTANT_TIME_CONFIG, instantTime);
}
- BuiltinKeyGenerator builtinKeyGenerator = (BuiltinKeyGenerator)
HoodieSparkKeyGeneratorFactory.createKeyGenerator(props);
+ BuiltinKeyGenerator builtinKeyGenerator = (BuiltinKeyGenerator)
HoodieSparkKeyGeneratorFactory.createKeyGenerator(
+
KeyGenUtils.withComplexKeyGenEncoding(TypedProperties.copy(props),
tableConfig));
Review Comment:
Audited every `createKeyGenerator` call site. The six that key records apply
`withComplexKeyGenEncoding`, and the rest are either partition path only
(`HoodieSparkSqlWriter` 266 and 450, `SparkKeyGenUtils`, and
`HoodieIndexUtils#inferPartitionPath`, whose record key comes from the incoming
record) or reached only when `populateMetaFields` is false, where the encoding
is not tracked. `BucketIndexSupport` arrives there a third way, merging the
table config into the props itself, so what is missing is a single idiom rather
than a fix.
--
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]
