hudi-agent commented on code in PR #19206:
URL: https://github.com/apache/hudi/pull/19206#discussion_r3686863952


##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java:
##########
@@ -1522,6 +1546,28 @@ public Properties build() {
       if (null != populateMetaFields) {
         tableConfig.setValue(HoodieTableConfig.POPULATE_META_FIELDS, 
Boolean.toString(populateMetaFields));
       }
+      // Persist the mode in one place. Rules:
+      //  - Explicit selective mode wins → validate compatibility with 
populateMetaFields and write.
+      //  - populateMetaFields=false + no explicit mode → NONE (leave property 
empty for backward compat).
+      //  - populateMetaFields=true + no explicit mode → ALL (implicit; leave 
property empty).
+      if (null != metaFieldsMode) {

Review Comment:
   🤖 If a selective `metaFieldsMode` is set while `populateMetaFields` is still 
null (e.g. a properties source that has `hoodie.meta.fields.mode` but not 
`hoodie.populate.meta.fields`, since `fromProperties` only calls 
`setPopulateMetaFields` when the key is present), neither guard fires: the 
selective mode gets persisted but `POPULATE_META_FIELDS` is never written (line 
1546). On read, `populateMetaFields()` defaults to true, so 
`getMetaFieldsMode()` resolves back to ALL and the selective mode is silently 
dropped — leaving hoodie.properties claiming e.g. COMMIT_TIME_ONLY while 
writers actually run in ALL mode. Should build() default 
`populateMetaFields=false` (or reject) when a selective mode is set without an 
explicit populate flag?
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag 
quality.</i></sub>



##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/io/storage/row/HoodieRowCreateHandle.java:
##########
@@ -158,10 +159,45 @@ public HoodieRowCreateHandle(HoodieTable table,
    * @throws IOException
    */
   public void write(InternalRow row) throws IOException {
-    if (populateMetaFields) {
-      writeRow(row);
-    } else {
-      writeRowNoMetaFields(row);
+    switch (metaFieldsMode) {
+      case ALL:
+        writeRow(row);
+        break;
+      case NONE:
+        writeRowNoMetaFields(row);
+        break;
+      default:
+        writeRowSelectiveMetaFields(row);
+        break;
+    }
+  }
+
+  /**
+   * Selective meta-field write path: populate only the meta columns opted in 
via
+   * {@code hoodie.meta.fields.mode} — {@code _hoodie_commit_time} and/or 
{@code _hoodie_file_name}.
+   * The other meta columns stay null on disk. Record key is never populated 
in this path, so the
+   * record key is not registered with the write support (bloom filter / RLI 
hooks are meaningless
+   * without the record-key column).
+   */
+  private void writeRowSelectiveMetaFields(InternalRow row) {

Review Comment:
   🤖 nit: the magic `5` here is the number of Hoodie meta columns — could you 
use `HoodieRecord.HOODIE_META_COLUMNS.size()` (or a named constant) so a future 
reader doesn't have to count the fields to understand where the number comes 
from?
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag 
quality.</i></sub>



##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/io/storage/HoodieSparkFileWriterFactory.java:
##########
@@ -57,6 +57,9 @@ protected HoodieFileWriter newParquetFileWriter(
       String instantTime, StoragePath path, HoodieConfig config, HoodieSchema 
schema,
       TaskContextSupplier taskContextSupplier) throws IOException {

Review Comment:
   🤖 nit: `MetaFieldsMode` is referenced with its full package name here — 
could you add it to the import block at the top of the file instead? The same 
pattern is used in `HoodieAvroFileWriterFactory` (line 71).
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag 
quality.</i></sub>



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