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


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/keygen/KeyGenUtils.java:
##########
@@ -436,15 +461,213 @@ 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 = 
props.getProperty(HoodieTableConfig.COMPLEX_KEYGEN_ENCODING.key());

Review Comment:
   If `encodeSingleKeyFieldNameForComplexKeyGen` is only used by the key 
generator now, the method should be protected in `BaseKeyGenerator` to avoid 
any other usage.



##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/keygen/KeyGenUtils.java:
##########
@@ -436,15 +461,213 @@ 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 = 
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);
   }
 
-  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 isComplexKeyGenEncodingTracked(HoodieTableConfig 
tableConfig) {

Review Comment:
   The naming is a bit confusing.  A better name 
`requireComplexKeyGenEncodingTracked` conveys the meaning.



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