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


##########
hudi-spark-datasource/hudi-spark/src/test/java/org/apache/hudi/functional/TestMetaFieldsModeE2E.java:
##########
@@ -0,0 +1,830 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hudi.functional;
+
+import org.apache.hudi.DataSourceReadOptions;
+import org.apache.hudi.DataSourceWriteOptions;
+import org.apache.hudi.SparkAdapterSupport$;
+import org.apache.hudi.common.config.HoodieMetadataConfig;
+import org.apache.hudi.common.model.HoodieRecord;
+import org.apache.hudi.common.model.MetaFieldsMode;
+import org.apache.hudi.common.table.HoodieTableConfig;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.testutils.SparkClientFunctionalTestHarness;
+
+import org.apache.spark.sql.Dataset;
+import org.apache.spark.sql.Row;
+import org.apache.spark.sql.RowFactory;
+import org.apache.spark.sql.SaveMode;
+import org.apache.spark.sql.functions;
+import org.apache.spark.sql.types.DataTypes;
+import org.apache.spark.sql.types.StructField;
+import org.apache.spark.sql.types.StructType;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Spark-datasource end-to-end tests for the {@code hoodie.meta.fields.mode} 
property on CoW tables.
+ * Every {@link MetaFieldsMode} value is exercised via a write / re-read round 
trip; on-disk column
+ * population is verified by reading the parquet files back and inspecting the 
meta-column values.
+ */
+class TestMetaFieldsModeE2E extends SparkClientFunctionalTestHarness {
+
+  private static StructType simpleSchema() {
+    return DataTypes.createStructType(new StructField[]{
+        DataTypes.createStructField("column1", DataTypes.StringType, true),
+        DataTypes.createStructField("column2", DataTypes.StringType, true),
+        DataTypes.createStructField("column3", DataTypes.StringType, true)
+    }).asNullable();
+  }
+
+  private Map<String, String> baseOptions() {
+    Map<String, String> opts = new HashMap<>();
+    opts.put(DataSourceWriteOptions.RECORDKEY_FIELD().key(), "column1");
+    opts.put(DataSourceWriteOptions.PARTITIONPATH_FIELD().key(), "column2");
+    opts.put(DataSourceWriteOptions.ORDERING_FIELDS().key(), "column3");
+    opts.put(HoodieTableConfig.NAME.key(), "test_meta_fields_mode");
+    opts.put(DataSourceWriteOptions.TABLE_TYPE().key(), "COPY_ON_WRITE");
+    opts.put(HoodieMetadataConfig.ENABLE.key(), "false");
+    return opts;
+  }
+
+  private void writeRows(List<Row> records, StructType schema, Map<String, 
String> options, String path, SaveMode mode) {
+    spark().createDataset(records,
+            
SparkAdapterSupport$.MODULE$.sparkAdapter().getCatalystExpressionUtils().getEncoder(schema))
+        .write()
+        .format("hudi")
+        .options(options)
+        .mode(mode)
+        .save(path);
+  }
+
+  private HoodieTableConfig writeSampleAndGetTableConfig(Map<String, String> 
options, String path) {
+    writeRows(Arrays.asList(
+            RowFactory.create("k1", "p1", "v1"),
+            RowFactory.create("k2", "p1", "v2")),
+        simpleSchema(), options, path, SaveMode.Overwrite);
+    HoodieTableMetaClient metaClient =
+        
HoodieTableMetaClient.builder().setBasePath(path).setConf(storageConf()).build();
+    return metaClient.getTableConfig();
+  }
+
+  /**
+   * End-to-end assertion of the on-disk meta columns after a write. Reads the 
parquet files back
+   * (bypassing Hudi's own read path so we see the raw column values) and 
asserts which meta
+   * columns are non-null.
+   */
+  private void assertMetaColumnPopulation(String path, MetaFieldsMode 
expectedMode) {
+    Dataset<Row> raw = spark().read().parquet(path + "/*/*.parquet");
+    Row first = raw.select(
+        HoodieRecord.COMMIT_TIME_METADATA_FIELD,
+        HoodieRecord.COMMIT_SEQNO_METADATA_FIELD,
+        HoodieRecord.RECORD_KEY_METADATA_FIELD,
+        HoodieRecord.PARTITION_PATH_METADATA_FIELD,
+        HoodieRecord.FILENAME_METADATA_FIELD).first();
+
+    if (expectedMode.isCommitTimePopulated()) {
+      assertNotNull(first.get(0), "expected _hoodie_commit_time to be 
populated for mode " + expectedMode);
+    } else {

Review Comment:
   🤖 nit: `first.get(0)` / `first.get(4)` etc. are positional and fragile — if 
the `select()` columns are ever reordered the assertions silently check the 
wrong fields. Could you use 
`first.getAs(HoodieRecord.COMMIT_TIME_METADATA_FIELD)` (and the other 
field-name constants) instead? The same pattern appears a few lines below and 
again in `assertOnDiskMetaColumns` in the streamer test.
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag 
quality.</i></sub>



##########
hudi-utilities/src/test/java/org/apache/hudi/utilities/deltastreamer/TestHoodieStreamerMetaFieldsMode.java:
##########
@@ -0,0 +1,285 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hudi.utilities.deltastreamer;
+
+import org.apache.hudi.common.model.HoodieRecord;
+import org.apache.hudi.common.model.MetaFieldsMode;
+import org.apache.hudi.common.model.WriteOperationType;
+import org.apache.hudi.common.table.HoodieTableConfig;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.testutils.HoodieTestUtils;
+
+import org.apache.spark.sql.Dataset;
+import org.apache.spark.sql.Row;
+import org.apache.spark.sql.functions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * End-to-end coverage for {@code hoodie.meta.fields.mode} through the 
HoodieStreamer entrypoint.
+ * Each parameterized invocation runs a single ingest cycle in the given 
{@link MetaFieldsMode} and
+ * verifies both the persisted table property and the actual on-disk parquet 
column population.
+ *
+ * <p>Rejection paths (unknown token, populate=true+mode, MoR+mode) are 
exercised in the datasource
+ * test {@code TestMetaFieldsMode}; this fixture focuses on the streamer 
control-flow.
+ */
+public class TestHoodieStreamerMetaFieldsMode extends 
HoodieDeltaStreamerTestBase {
+
+  /**
+   * Only the selective modes are parameterized here. ALL and NONE add no mode 
key at all, so they
+   * exercise none of the streamer-side plumbing this feature introduced — 
they are covered by the
+   * datasource tests and by {@code TestHoodieTableConfig}'s resolution cases.
+   */
+  @ParameterizedTest
+  @EnumSource(value = MetaFieldsMode.class,
+      names = {"COMMIT_TIME_ONLY", "FILE_NAME_ONLY", 
"COMMIT_TIME_AND_FILE_NAME"})
+  public void testStreamerRespectsMetaFieldsMode(MetaFieldsMode mode) throws 
Exception {
+    String tablePath = basePath + "/streamer_meta_fields_mode_" + mode.name();
+    HoodieDeltaStreamer.Config cfg = TestHelpers.makeConfig(tablePath, 
WriteOperationType.INSERT);
+    // Force CoW; selective modes are CoW-only until MoR log-write is wired.
+    cfg.tableType = "COPY_ON_WRITE";
+    // The mode alone — pairing it with populate.meta.fields would now be a 
stated conflict, since the
+    // mode is authoritative and the boolean is only the fallback for 
resolving an absent one.
+    cfg.configs.add(HoodieTableConfig.META_FIELDS_MODE.key() + "=" + 
mode.name());
+    HoodieDeltaStreamer streamer = new HoodieDeltaStreamer(cfg, jsc);
+    streamer.getIngestionService().ingestOnce();
+    streamer.shutdownGracefully();
+
+    HoodieTableMetaClient metaClient = 
HoodieTestUtils.createMetaClient(context, tablePath);
+    assertEquals(mode, metaClient.getTableConfig().getMetaFieldsMode(),
+        "streamer must persist mode=" + mode + " on hoodie.properties");
+    assertOnDiskMetaColumns(tablePath, mode);
+  }
+
+  /**
+   * The regression this fixture exists for, and the one cshuo raised in 
review: a restarted streamer
+   * that does not restate the mode must not silently write the wrong meta 
columns.
+   *
+   * <p>StreamSync builds its write config from {@code props} alone, and the 
mode is persisted only by
+   * {@code initializeEmptyTable}, which runs solely when the base path does 
not exist. So a second run
+   * used to resolve to {@link MetaFieldsMode#NONE} and write base files with 
a null
+   * {@code _hoodie_commit_time} while {@code hoodie.properties} still 
advertised
+   * {@code COMMIT_TIME_ONLY} — incremental queries were then admitted and 
silently dropped every one
+   * of those rows.
+   *
+   * <p>A selective table now requires the writer to state the mode, so the 
restart is <b>rejected</b>
+   * rather than inheriting. Inheritance would work here, but it cannot be 
relied on everywhere: the
+   * write path reads the mode in factories and handles that hold no table 
config at all, so the write
+   * config has to be right on its own. Failing loudly at init is what makes 
that guarantee real —
+   * and a rejected run leaves the table exactly as it was, which silent 
narrowing did not.
+   * See {@code testRestartRestatingTheModeSucceeds} for the migration path.
+   */
+  @Test
+  public void testRestartWithoutRestatingTheModeIsRejected() throws Exception {
+    String tablePath = basePath + "/streamer_restart_inherits_mode";
+
+    HoodieDeltaStreamer.Config first = TestHelpers.makeConfig(tablePath, 
WriteOperationType.INSERT);
+    first.tableType = "COPY_ON_WRITE";
+    first.configs.add(HoodieTableConfig.META_FIELDS_MODE.key() + "=" + 
MetaFieldsMode.COMMIT_TIME_ONLY.name());
+    HoodieDeltaStreamer streamer = new HoodieDeltaStreamer(first, jsc);
+    streamer.getIngestionService().ingestOnce();
+    streamer.shutdownGracefully();
+
+    assertEquals(MetaFieldsMode.COMMIT_TIME_ONLY,
+        HoodieTestUtils.createMetaClient(context, 
tablePath).getTableConfig().getMetaFieldsMode());
+
+    long commitsAfterFirstRun = HoodieTestUtils.createMetaClient(context, 
tablePath)
+        .getActiveTimeline().filterCompletedInstants().countInstants();
+
+    // Restart against the existing table stating neither the mode nor the 
legacy boolean.
+    HoodieDeltaStreamer.Config restart = TestHelpers.makeConfig(tablePath, 
WriteOperationType.INSERT);
+    restart.tableType = "COPY_ON_WRITE";
+
+    Throwable thrown = assertThrows(Throwable.class, () -> {
+      HoodieDeltaStreamer restarted = new HoodieDeltaStreamer(restart, jsc);
+      restarted.getIngestionService().ingestOnce();
+      restarted.shutdownGracefully();
+    });
+
+    String rootMessage = rootMessageOf(thrown);
+    assertTrue(rootMessage.contains(HoodieTableConfig.META_FIELDS_MODE.key()),
+        "expected the writer to be told to state the mode, got: " + 
rootMessage);
+
+    // The rejected run must have left the table untouched -- no mode change, 
no extra commit, and no
+    // base file carrying a null commit time. That last one is the actual data 
loss being prevented:
+    // such rows are admitted by incremental queries and then silently dropped.
+    HoodieTableMetaClient metaClient = 
HoodieTestUtils.createMetaClient(context, tablePath);
+    assertEquals(MetaFieldsMode.COMMIT_TIME_ONLY, 
metaClient.getTableConfig().getMetaFieldsMode(),
+        "a rejected restart must leave the table's mode untouched");
+    assertEquals(commitsAfterFirstRun,
+        
metaClient.getActiveTimeline().filterCompletedInstants().countInstants(),
+        "a rejected restart must not have committed anything");
+    Dataset<Row> raw = sparkSession.read().parquet(tablePath + 
"/*/*/*/*.parquet");
+    assertEquals(0,
+        
raw.filter(functions.col(HoodieRecord.COMMIT_TIME_METADATA_FIELD).isNull()).count(),
+        "no row may have a null _hoodie_commit_time on a COMMIT_TIME_ONLY 
table");
+  }
+
+  /**
+   * The migration path for the case above: a restart that restates the 
table's mode proceeds normally.
+   *
+   * <p>This is what every writer against a selective table must do, and it is 
the assertion that keeps
+   * the requirement from being a dead end -- the mode is stateable, and 
stating it is enough.
+   */
+  @Test
+  public void testRestartRestatingTheModeSucceeds() throws Exception {
+    String tablePath = basePath + "/streamer_restart_restates_mode";
+
+    HoodieDeltaStreamer.Config first = TestHelpers.makeConfig(tablePath, 
WriteOperationType.INSERT);
+    first.tableType = "COPY_ON_WRITE";
+    first.configs.add(HoodieTableConfig.META_FIELDS_MODE.key() + "=" + 
MetaFieldsMode.COMMIT_TIME_ONLY.name());
+    HoodieDeltaStreamer streamer = new HoodieDeltaStreamer(first, jsc);
+    streamer.getIngestionService().ingestOnce();
+    streamer.shutdownGracefully();
+
+    // Restart restating the same mode.
+    HoodieDeltaStreamer.Config restart = TestHelpers.makeConfig(tablePath, 
WriteOperationType.INSERT);
+    restart.tableType = "COPY_ON_WRITE";
+    restart.configs.add(HoodieTableConfig.META_FIELDS_MODE.key() + "=" + 
MetaFieldsMode.COMMIT_TIME_ONLY.name());
+    HoodieDeltaStreamer restarted = new HoodieDeltaStreamer(restart, jsc);
+    restarted.getIngestionService().ingestOnce();
+    restarted.shutdownGracefully();
+
+    HoodieTableMetaClient metaClient = 
HoodieTestUtils.createMetaClient(context, tablePath);
+    assertEquals(MetaFieldsMode.COMMIT_TIME_ONLY, 
metaClient.getTableConfig().getMetaFieldsMode(),
+        "the restart must not have changed the table's mode");
+    
assertTrue(metaClient.getActiveTimeline().filterCompletedInstants().countInstants()
 >= 2,
+        "expected the restart to have produced a second commit");
+
+    // Every row across both commits carries a commit time. A row with a null 
one is what incremental
+    // queries silently drop, so this is the assertion that catches the 
regression.
+    Dataset<Row> raw = sparkSession.read().parquet(tablePath + 
"/*/*/*/*.parquet");
+    assertEquals(0,
+        
raw.filter(functions.col(HoodieRecord.COMMIT_TIME_METADATA_FIELD).isNull()).count(),
+        "no row may have a null _hoodie_commit_time on a COMMIT_TIME_ONLY 
table");
+    
assertTrue(raw.select(HoodieRecord.COMMIT_TIME_METADATA_FIELD).distinct().count()
 >= 2,
+        "both commits must be represented, so the second run really did write 
through this path");
+  }
+
+  /**
+   * The variant @voonhous asked for, and the one cshuo originally described: 
the restart states the
+   * deprecated boolean rather than nothing at all.
+   *
+   * <p>Both cases are rejected against a selective table, for the same 
reason: the writer has not
+   * stated the mode the table is on. Stating the boolean instead makes the 
disagreement explicit
+   * rather than merely unstated, but the outcome is the same. Before this 
rule it narrowed silently,
+   * writing base files with a null {@code _hoodie_commit_time} into a table 
that still advertised the
+   * mode.
+   */
+  @Test
+  public void testRestartStatingTheLegacyBooleanIsRejected() throws Exception {
+    String tablePath = basePath + "/streamer_restart_legacy_boolean_conflict";
+
+    HoodieDeltaStreamer.Config first = TestHelpers.makeConfig(tablePath, 
WriteOperationType.INSERT);
+    first.tableType = "COPY_ON_WRITE";
+    first.configs.add(HoodieTableConfig.META_FIELDS_MODE.key() + "=" + 
MetaFieldsMode.COMMIT_TIME_ONLY.name());
+    HoodieDeltaStreamer streamer = new HoodieDeltaStreamer(first, jsc);
+    streamer.getIngestionService().ingestOnce();
+    streamer.shutdownGracefully();
+
+    HoodieDeltaStreamer.Config restart = TestHelpers.makeConfig(tablePath, 
WriteOperationType.INSERT);
+    restart.tableType = "COPY_ON_WRITE";
+    restart.configs.add(HoodieTableConfig.POPULATE_META_FIELDS.key() + 
"=false");
+
+    Throwable thrown = assertThrows(Throwable.class, () -> {
+      HoodieDeltaStreamer restarted = new HoodieDeltaStreamer(restart, jsc);
+      restarted.getIngestionService().ingestOnce();
+      restarted.shutdownGracefully();
+    });
+
+    String rootMessage = rootMessageOf(thrown);
+    assertTrue(rootMessage.contains(HoodieTableConfig.META_FIELDS_MODE.key())
+            || 
rootMessage.contains(HoodieTableConfig.POPULATE_META_FIELDS.key()),
+        "expected a meta-fields conflict, got: " + rootMessage);
+
+    // The failed run must not have changed the table, nor written rows with a 
null commit time.
+    HoodieTableMetaClient metaClient = 
HoodieTestUtils.createMetaClient(context, tablePath);
+    assertEquals(MetaFieldsMode.COMMIT_TIME_ONLY, 
metaClient.getTableConfig().getMetaFieldsMode(),
+        "a rejected restart must leave the table's mode untouched");
+    Dataset<Row> raw = sparkSession.read().parquet(tablePath + 
"/*/*/*/*.parquet");
+    assertEquals(0, 
raw.filter(functions.col(HoodieRecord.COMMIT_TIME_METADATA_FIELD).isNull()).count(),
+        "no row may have a null _hoodie_commit_time");
+  }
+
+  @Test
+  public void testStreamerRejectsMorWithSelectiveMode() throws Exception {
+    String tablePath = basePath + "/streamer_mor_selective_rejected";
+    HoodieDeltaStreamer.Config cfg = TestHelpers.makeConfig(tablePath, 
WriteOperationType.BULK_INSERT);
+    cfg.tableType = "MERGE_ON_READ";
+    cfg.configs.add(HoodieTableConfig.META_FIELDS_MODE.key() + "=" + 
MetaFieldsMode.COMMIT_TIME_ONLY.name());
+
+    Throwable thrown = assertThrows(Throwable.class, () -> {
+      HoodieDeltaStreamer streamer = new HoodieDeltaStreamer(cfg, jsc);
+      streamer.getIngestionService().ingestOnce();
+      streamer.shutdownGracefully();
+    });
+
+    String rootMessage = rootMessageOf(thrown);
+    assertTrue(rootMessage.contains("COPY_ON_WRITE") || 
rootMessage.contains("MERGE_ON_READ")
+            || rootMessage.contains("MoR") || 
rootMessage.contains(HoodieTableConfig.META_FIELDS_MODE.key()),
+        "Expected MoR-restriction error, got: " + rootMessage);
+  }
+
+  private void assertOnDiskMetaColumns(String tablePath, MetaFieldsMode 
expectedMode) {
+    // Default HoodieTestDataGenerator partitions are YYYY/MM/DD (three 
levels).
+    Dataset<Row> raw = sparkSession.read().parquet(tablePath + 
"/*/*/*/*.parquet");
+    Row first = raw.select(
+        HoodieRecord.COMMIT_TIME_METADATA_FIELD,
+        HoodieRecord.COMMIT_SEQNO_METADATA_FIELD,
+        HoodieRecord.RECORD_KEY_METADATA_FIELD,
+        HoodieRecord.PARTITION_PATH_METADATA_FIELD,
+        HoodieRecord.FILENAME_METADATA_FIELD).first();
+
+    if (expectedMode.isCommitTimePopulated()) {
+      assertNotNull(first.get(0), "commit_time must be populated in mode " + 
expectedMode);
+    } else {
+      assertNull(first.get(0), "commit_time must be null in mode " + 
expectedMode);
+    }
+    if (expectedMode.isFileNamePopulated()) {
+      assertNotNull(first.get(4), "file_name must be populated in mode " + 
expectedMode);
+    } else {
+      assertNull(first.get(4), "file_name must be null in mode " + 
expectedMode);
+    }
+    if (expectedMode == MetaFieldsMode.ALL) {
+      assertNotNull(first.get(1), "commit_seq_no must be populated in ALL 
mode");
+      assertNotNull(first.get(2), "record_key must be populated in ALL mode");
+      assertNotNull(first.get(3), "partition_path must be populated in ALL 
mode");
+    } else {
+      assertNull(first.get(1), "commit_seq_no must be null outside ALL mode");
+      assertNull(first.get(2), "record_key must be null outside ALL mode");
+      assertNull(first.get(3), "partition_path must be null outside ALL mode");
+    }
+  }
+
+  private static String rootMessageOf(Throwable thrown) {

Review Comment:
   🤖 nit: `rootMessageOf` is copy-pasted verbatim from `TestMetaFieldsModeE2E`. 
Could it live in a shared test-util base class or a static helper in the 
existing `TestHelpers`? Otherwise the two copies will drift.
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag 
quality.</i></sub>



##########
hudi-hadoop-common/src/main/java/org/apache/hudi/io/storage/hadoop/HoodieAvroFileWriterFactory.java:
##########
@@ -123,7 +125,13 @@ protected HoodieFileWriter newHFileFileWriter(
         HoodieAvroHFileReaderImplBase.KEY_FIELD_NAME,
         filter,
         config.getBoolean(HFILE_WRITER_TO_ALLOW_DUPLICATES));
-    return new HoodieAvroHFileWriter(instantTime, path, hfileConfig, schema, 
taskContextSupplier, config.getBoolean(HoodieTableConfig.POPULATE_META_FIELDS));
+    // Resolve through the mode like the parquet path above. HFile does not 
populate meta columns
+    // selectively, so a selective mode is treated as "record key not 
populated". Note getBoolean
+    // (unlike getBooleanOrDefault) returns null when neither property is set, 
which would NPE here.
+    boolean populateMetaFields = 
org.apache.hudi.common.model.MetaFieldsMode.resolve(

Review Comment:
   🤖 nit: `MetaFieldsMode` is already imported at the top of this file — could 
you drop the fully-qualified 
`org.apache.hudi.common.model.MetaFieldsMode.resolve(...)` and just use 
`MetaFieldsMode.resolve(...)`? The FQN makes this line look like a one-off 
workaround when it's actually the same pattern used in the parquet path above.
   
   <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:
##########
@@ -119,7 +121,12 @@ protected HoodieFileWriter newOrcFileWriter(String 
instantTime, StoragePath path
   protected HoodieFileWriter newLanceFileWriter(String instantTime, 
StoragePath path, HoodieConfig config, HoodieSchema schema,
                                                 TaskContextSupplier 
taskContextSupplier) throws IOException {
     HoodieSparkLanceWriter.validateNoVariantColumns(schema);
-    boolean populateMetaFields = 
config.getBooleanOrDefault(HoodieTableConfig.POPULATE_META_FIELDS);
+    // Resolve through hoodie.meta.fields.mode rather than the deprecated 
boolean — see the parquet
+    // path above. Lance does not yet populate meta columns selectively, so a 
selective mode is
+    // treated as "record key not populated" (no bloom filter, no meta 
stamping).
+    boolean populateMetaFields = 
org.apache.hudi.common.model.MetaFieldsMode.resolve(

Review Comment:
   🤖 nit: `MetaFieldsMode` is already imported — could you use the simple name 
here (and on the Vortex path at line 157 below) instead of the fully-qualified 
`org.apache.hudi.common.model.MetaFieldsMode.resolve(...)`? The parquet path 
just above uses the simple name, so this looks like an accidental inconsistency.
   
   <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