nsivabalan commented on code in PR #19205:
URL: https://github.com/apache/hudi/pull/19205#discussion_r3654553594


##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/io/storage/HoodieSparkParquetWriter.java:
##########
@@ -55,11 +56,20 @@ public HoodieSparkParquetWriter(StoragePath file,
                                   String instantTime,
                                   TaskContextSupplier taskContextSupplier,
                                   boolean populateMetaFields) throws 
IOException {
+    this(file, parquetConfig, instantTime, taskContextSupplier,

Review Comment:
   You're right — removed in 
[`7dcd9d1`](https://github.com/apache/hudi/pull/19205/commits/7dcd9d1b77f5). 
That overload had **no callers at all**: the only construction site, 
`HoodieSparkFileWriterFactory`, already passes a `MetaFieldsMode`. I'd added it 
for symmetry with the Avro writer without checking whether anything needed it. 
Gone.



##########
hudi-hadoop-common/src/main/java/org/apache/hudi/io/storage/hadoop/HoodieAvroParquetWriter.java:
##########
@@ -57,30 +60,57 @@ public HoodieAvroParquetWriter(StoragePath file,
                                  String instantTime,
                                  TaskContextSupplier taskContextSupplier,
                                  boolean populateMetaFields) throws 
IOException {
+    this(file, parquetConfig, instantTime, taskContextSupplier,
+        populateMetaFields ? MetaFieldsMode.ALL : MetaFieldsMode.NONE);
+  }
+
+  @SuppressWarnings({"unchecked", "rawtypes"})
+  public HoodieAvroParquetWriter(StoragePath file,

Review Comment:
   This one I'd like to keep, though I've marked it deprecated in 
[`7dcd9d1`](https://github.com/apache/hudi/pull/19205/commits/7dcd9d1b77f5).
   
   Unlike the Spark writer's equivalent — which I removed in the same commit 
after your other comment, since it had zero callers — this overload has a live 
production caller at `SparkHelpers.scala:74` plus four test call sites 
(`TestHoodieAvroParquetWriter`, `HoodieWriteableTestTable`, 
`TestSparkBinaryCopyClusteringAndValidationMeta`, `HoodieFileSliceTestUtils`). 
All of them genuinely only distinguish all-or-nothing meta fields, so 
converting them to `MetaFieldsMode.ALL` / `NONE` would be churn without 
changing behavior.
   
   It's now `@Deprecated` with a javadoc pointing at the `MetaFieldsMode` 
overload and noting it cannot express the selective modes, so new callers get 
steered correctly and the existing ones can migrate separately. Happy to 
convert them in this PR instead if you'd prefer the boolean gone entirely.



##########
hudi-common/src/main/java/org/apache/hudi/common/model/MetaFieldsMode.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.common.model;
+
+import org.apache.hudi.common.util.StringUtils;
+
+/**
+ * Which of Hudi's meta columns are physically populated on disk.
+ *
+ * <p>Selective modes exist so that tables that opt out of the default {@code 
populate.meta.fields=true}
+ * can still keep the two columns that matter for downstream operations 
without paying for the other
+ * three:
+ *
+ * <ul>
+ *   <li>{@code _hoodie_commit_time} — required for incremental queries.</li>
+ *   <li>{@code _hoodie_file_name} — useful for file-level pruning / 
investigation lookups.</li>
+ * </ul>
+ *
+ * <p>The remaining three meta columns ({@code _hoodie_commit_seqno}, {@code 
_hoodie_record_key},
+ * {@code _hoodie_partition_path}) are all-or-nothing — either populate every 
meta column ({@link #ALL})
+ * or none of them beyond the two selectable ones. If you need any of the 
remaining columns, set
+ * {@code hoodie.populate.meta.fields=true}.
+ *
+ * <p>This enum is the single source of truth for meta-column population. The 
legacy boolean
+ * {@code hoodie.populate.meta.fields} is deprecated and consulted only when
+ * {@code hoodie.meta.fields.mode} is absent, so that tables written before 
the mode property
+ * existed keep their behavior:
+ *
+ * <ul>
+ *   <li>{@code populate.meta.fields=true} (or absent) → {@link #ALL} — 
today's default.</li>
+ *   <li>{@code populate.meta.fields=false} → {@link #NONE}.</li>
+ * </ul>
+ *
+ * <p>On-disk representation: the enum {@link #name()} is persisted in {@code 
hoodie.properties}
+ * under the property {@code hoodie.meta.fields.mode}.
+ */
+public enum MetaFieldsMode {
+  /**
+   * All five Hudi meta columns are populated — today's default.
+   */
+  ALL(true, true),
+
+  /**
+   * No Hudi meta columns are populated. Incremental queries are unsupported. 
File-level pruning
+   * that depends on {@code _hoodie_file_name} is unsupported.
+   */
+  NONE(false, false),
+
+  /**
+   * Only {@code _hoodie_commit_time} is populated. Incremental queries remain 
functional; other
+   * meta columns stay null on disk.
+   */
+  COMMIT_TIME_ONLY(true, false),
+
+  /**
+   * Only {@code _hoodie_file_name} is populated. Useful for file-level 
lookups and debugging;
+   * incremental queries are unsupported.
+   */
+  FILE_NAME_ONLY(false, true),
+
+  /**
+   * Both {@code _hoodie_commit_time} and {@code _hoodie_file_name} are 
populated.
+   */
+  COMMIT_TIME_AND_FILE_NAME(true, true);
+
+  private final boolean commitTimePopulated;
+  private final boolean fileNamePopulated;
+
+  MetaFieldsMode(boolean commitTimePopulated, boolean fileNamePopulated) {
+    this.commitTimePopulated = commitTimePopulated;
+    this.fileNamePopulated = fileNamePopulated;
+  }
+
+  public boolean isCommitTimePopulated() {
+    return commitTimePopulated;
+  }
+
+  public boolean isFileNamePopulated() {
+    return fileNamePopulated;
+  }
+
+  /**
+   * @return true when all five meta columns are populated (i.e. this is 
{@link #ALL}). Selective
+   * modes never populate {@code _hoodie_record_key}, {@code 
_hoodie_partition_path}, or
+   * {@code _hoodie_commit_seqno}.
+   */
+  public boolean isRecordKeyPopulated() {
+    return this == ALL;
+  }
+
+  /**
+   * Resolve the effective mode. {@code hoodie.meta.fields.mode} is the source 
of truth; the
+   * deprecated {@code hoodie.populate.meta.fields} boolean is a fallback for 
tables written before
+   * the mode property existed. Precedence:
+   *
+   * <ul>
+   *   <li>non-empty mode → the parsed enum value (the legacy boolean is not 
consulted).</li>
+   *   <li>null/empty mode + {@code populateMetaFields=false} → {@link 
#NONE}.</li>
+   *   <li>null/empty mode + {@code populateMetaFields=true} → {@link 
#ALL}.</li>
+   * </ul>
+   *
+   * @param rawMode             raw {@code hoodie.meta.fields.mode} value; may 
be null or empty.
+   * @param legacyPopulateMetaFields value of the deprecated {@code 
hoodie.populate.meta.fields}.
+   * @throws IllegalArgumentException when the raw mode value does not match 
any enum value. This
+   *         includes the pre-enum comma-separated format — callers that 
upgrade an old table must
+   *         migrate the value through the hudi-cli.
+   */
+  public static MetaFieldsMode resolve(String rawMode, boolean 
legacyPopulateMetaFields) {
+    if (StringUtils.isNullOrEmpty(rawMode)) {
+      return legacyPopulateMetaFields ? ALL : NONE;
+    }
+    return parse(rawMode);
+  }
+
+  /**
+   * Parse a raw {@code hoodie.meta.fields.mode} value into an enum constant, 
with a message that
+   * lists the allowed values. Prefer this over {@link #valueOf(String)} for 
user-supplied input.
+   */
+  public static MetaFieldsMode parse(String rawMode) {
+    try {
+      return MetaFieldsMode.valueOf(rawMode.trim());

Review Comment:
   Done in 
[`7dcd9d1`](https://github.com/apache/hudi/pull/19205/commits/7dcd9d1b77f5) — 
`parse` now upper-cases with `Locale.ROOT` before `valueOf`, so a hand-edited 
`hoodie.properties` or a write option doesn't have to match the enum's casing. 
`TestHoodieMetaFieldsMode` covers lower-case, mixed-case and whitespace-padded 
forms.



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