shangxinli commented on code in PR #20282:
URL: https://github.com/apache/hudi/pull/20282#discussion_r4297506819


##########
hudi-common/src/main/java/org/apache/hudi/common/model/MetaFieldsMode.java:
##########
@@ -184,9 +184,12 @@ private static MetaFieldsMode parse(String rawMode) {
   /**
    * @return the equivalent value of the deprecated {@code 
hoodie.populate.meta.fields} boolean, so
    * that call sites not yet migrated to this enum keep observing consistent 
behavior.
+   *
+   * <p>Only {@link #NONE} maps to {@code false}: the boolean says whether any 
meta column is
+   * populated, not which. Use {@link #isRecordKeyPopulated()} for 
key-dependent checks.
    */
   public boolean toLegacyPopulateMetaFields() {
-    return this == ALL;
+    return this != NONE;

Review Comment:
   **major:** `isRecordKeyPopulated()` is still `this == ALL`, and its javadoc 
says selective modes never populate `_hoodie_record_key`, so after this flip 
the two contradict each other and none of the ~40 call sites still on the 
legacy boolean are migrated. `BaseHoodieLogRecordReader:192` then sets 
`recordKeyField` to `_hoodie_record_key` on a `COMMIT_TIME_ONLY` table, and 
`HoodieAppendHandle:442` writes the key, partition path and commit seqno the 
mode exists to suppress. It is persisted too: `HoodieTableConfig.create():702` 
now stores `hoodie.populate.meta.fields=true` for selective-mode tables below 
v10.



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