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


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -3587,11 +3632,102 @@ public Builder withCanIgnorePostCommitFailures(boolean 
canIgnorePostCommitFailur
       return this;
     }
 
+    /**

Review Comment:
   🤖 nit: this `@deprecated` Javadoc looks orphaned — it sits above the 
`statedPopulateMetaFields` field but its content clearly documents 
`withPopulateMetaFields`. Could you move it down onto that method so the field 
just keeps its own doc block?
   
   <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 +161,48 @@ 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) {
+    try {
+      UTF8String[] metaFields = new UTF8String[5];
+      if (metaFieldsMode.isCommitTimePopulated()) {
+        metaFields[HoodieRecord.COMMIT_TIME_METADATA_FIELD_ORD] = 
shouldPreserveHoodieMetadata
+            ? row.getUTF8String(HoodieRecord.COMMIT_TIME_METADATA_FIELD_ORD) : 
commitTime;

Review Comment:
   🤖 nit: the bare `new UTF8String[5]` for the meta-column count could use a 
named constant (e.g. the size of HoodieRecord's meta-fields list) so it stays 
in sync if that set ever changes.
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag 
quality.</i></sub>



##########
hudi-cli/src/main/java/org/apache/hudi/cli/commands/SparkMain.java:
##########
@@ -581,20 +581,46 @@ protected static int 
upgradeOrDowngradeTable(JavaSparkContext jsc, String basePa
   }
 
   private static SparkRDDWriteClient createHoodieClient(JavaSparkContext jsc, 
String basePath, Boolean rollbackUsingMarkers, boolean lazyCleanPolicy) throws 
Exception {
-    HoodieWriteConfig config = getWriteConfig(basePath, rollbackUsingMarkers, 
lazyCleanPolicy);
+    HoodieWriteConfig config = getWriteConfig(jsc, basePath, 
rollbackUsingMarkers, lazyCleanPolicy);
     return new SparkRDDWriteClient(new HoodieSparkEngineContext(jsc), config);
   }
 
   private static SparkRDDWriteClient createHoodieClient(JavaSparkContext jsc, 
String basePath, boolean lazyCleanPolicy) throws Exception {
     return createHoodieClient(jsc, basePath, 
Boolean.parseBoolean(HoodieWriteConfig.ROLLBACK_USING_MARKERS_ENABLE.defaultValue()),
 lazyCleanPolicy);
   }
 
-  private static HoodieWriteConfig getWriteConfig(String basePath, Boolean 
rollbackUsingMarkers, boolean lazyCleanPolicy) {
-    return HoodieWriteConfig.newBuilder().withPath(basePath)
+  private static HoodieWriteConfig getWriteConfig(JavaSparkContext jsc, String 
basePath, Boolean rollbackUsingMarkers, boolean lazyCleanPolicy) {
+    HoodieWriteConfig.Builder builder = 
HoodieWriteConfig.newBuilder().withPath(basePath)
         .withRollbackUsingMarkers(rollbackUsingMarkers)
         
.withCleanConfig(HoodieCleanConfig.newBuilder().withFailedWritesCleaningPolicy(lazyCleanPolicy
 ? HoodieFailedWritesCleaningPolicy.LAZY :
             HoodieFailedWritesCleaningPolicy.EAGER).build())
-        
.withIndexConfig(HoodieIndexConfig.newBuilder().withIndexType(HoodieIndex.IndexType.BLOOM).build()).build();
+        
.withIndexConfig(HoodieIndexConfig.newBuilder().withIndexType(HoodieIndex.IndexType.BLOOM).build());

Review Comment:
   🤖 Now that withMetaFieldsModeOf() copies the table's mode onto this builder, 
a selective (or virtual-key NONE) table reaches the new bloom check in 
validateAgainstTableProperties with this hardcoded BLOOM index. Since a 
selective table cannot have been created with a bloom index (creation-time 
validation rejects it), isRecordKeyPopulated() is false and the check throws -- 
e.g. deleteMarker() on a COMMIT_TIME_ONLY table returns -1. Should 
getWriteConfig read the table's actual index type instead of forcing BLOOM here?
   
   <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