yihua commented on code in PR #19304:
URL: https://github.com/apache/hudi/pull/19304#discussion_r4102269740


##########
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:
   User should not provide `COMPLEX_KEYGEN_ENCODING` as the write config.  Is 
this for table config or the variable naming is confusing (`HoodieConfig 
writeConfig`)?



##########
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:
   Should we remove `ConfigUtils.getBooleanWithAltKeys(props, 
COMPLEX_KEYGEN_NEW_ENCODING)`, given that `COMPLEX_KEYGEN_ENCODING` should be 
backfilled when this method is used?



-- 
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]

Reply via email to