nsivabalan commented on code in PR #19205:
URL: https://github.com/apache/hudi/pull/19205#discussion_r3707249076
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -3883,6 +3935,27 @@ private void validate() {
checkArgument(ttlStatsMaxParallelism > 0,
String.format("%s must be positive, but was %d",
HoodieTTLConfig.STATS_MAX_PARALLELISM.key(),
ttlStatsMaxParallelism));
+
+ // hoodie.meta.fields.mode is the source of truth for meta-column
population; the deprecated
+ // populate.meta.fields boolean is consulted only when the mode is
absent. There is therefore
+ // no ambiguous combination to reject here — MetaFieldsMode.resolve
throws on unrecognized
+ // values.
+ MetaFieldsMode metaFieldsMode = writeConfig.getMetaFieldsMode();
+ // Selective meta-field modes are CoW-only in this release. MoR
log-write path does not yet
+ // respect the mode, which would silently produce log records with null
meta columns.
+ boolean isSelective = metaFieldsMode != MetaFieldsMode.ALL &&
metaFieldsMode != MetaFieldsMode.NONE;
Review Comment:
Added in
[`5f397915d1d8`](https://github.com/apache/hudi/pull/19205/commits/5f397915d1d8).
`MetaFieldsMode.isSelective()` now replaces the open-coded `!= ALL && !=
NONE` at both sites that had it — the MoR / engine-type guards in
`HoodieWriteConfig.validate()` and the data-loss warning in
`TenToNineDowngradeHandler`.
I documented it as "the modes the deprecated boolean cannot express", since
that is the property every caller is actually gating on: a code path that only
understands all-or-nothing meta fields cannot speak for a five-state table.
Also added `TestMetaFieldsMode` under `hudi-common/.../common/model/`
covering it, including a round-trip check that a mode is non-selective exactly
when `toLegacyPopulateMetaFields` is lossless.
##########
hudi-common/src/main/java/org/apache/hudi/common/model/MetaFieldsMode.java:
##########
@@ -0,0 +1,191 @@
+/*
+ * 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.config.HoodieConfig;
+import org.apache.hudi.common.table.HoodieTableConfig;
+import org.apache.hudi.common.util.StringUtils;
+
+import java.util.Locale;
+
+/**
+ * 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.
+ */
+ /**
Review Comment:
Fixed in
[`5f397915d1d8`](https://github.com/apache/hudi/pull/19205/commits/5f397915d1d8)
— you read it exactly right, the block was left behind when `9495687a`
inserted the `resolve(HoodieConfig)` overload above it.
Moved the original block down onto `resolve(String, boolean)`, so the
precedence rules and the `@throws` contract are attached to the overload they
describe and the `HoodieConfig` overload keeps its own one-line javadoc.
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/io/storage/HoodieSparkFileWriterFactory.java:
##########
@@ -56,7 +56,9 @@ public HoodieSparkFileWriterFactory(HoodieStorage storage) {
protected HoodieFileWriter newParquetFileWriter(
String instantTime, StoragePath path, HoodieConfig config, HoodieSchema
schema,
TaskContextSupplier taskContextSupplier) throws IOException {
- boolean populateMetaFields =
config.getBooleanOrDefault(HoodieTableConfig.POPULATE_META_FIELDS);
+ org.apache.hudi.common.model.MetaFieldsMode metaFieldsMode =
Review Comment:
Fixed in
[`5f397915d1d8`](https://github.com/apache/hudi/pull/19205/commits/5f397915d1d8)
— imported in both `HoodieSparkFileWriterFactory` and
`HoodieAvroFileWriterFactory`, and the two `resolve(config)` call sites now use
the simple name.
Note the stacked PR #19378 adds two more fully-qualified references in this
same file (the Lance and Vortex paths). I will fold them into the simple name
there so the file does not end up mixed.
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/io/storage/HoodieSparkFileWriterFactory.java:
##########
@@ -56,7 +56,9 @@ public HoodieSparkFileWriterFactory(HoodieStorage storage) {
protected HoodieFileWriter newParquetFileWriter(
String instantTime, StoragePath path, HoodieConfig config, HoodieSchema
schema,
TaskContextSupplier taskContextSupplier) throws IOException {
- boolean populateMetaFields =
config.getBooleanOrDefault(HoodieTableConfig.POPULATE_META_FIELDS);
+ org.apache.hudi.common.model.MetaFieldsMode metaFieldsMode =
Review Comment:
Done in
[`5f397915d1d8`](https://github.com/apache/hudi/pull/19205/commits/5f397915d1d8)
— same fix as #19205 (comment) on this file; `MetaFieldsMode` is now imported
and used by simple name.
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/io/storage/hadoop/HoodieAvroFileWriterFactory.java:
##########
@@ -69,7 +69,9 @@ public HoodieAvroFileWriterFactory(HoodieStorage storage) {
protected HoodieFileWriter newParquetFileWriter(
String instantTime, StoragePath path, HoodieConfig config, HoodieSchema
schema,
TaskContextSupplier taskContextSupplier) throws IOException {
- boolean populateMetaFields =
config.getBooleanOrDefault(HoodieTableConfig.POPULATE_META_FIELDS);
+ org.apache.hudi.common.model.MetaFieldsMode metaFieldsMode =
Review Comment:
Done in
[`5f397915d1d8`](https://github.com/apache/hudi/pull/19205/commits/5f397915d1d8)
— import added here too.
##########
hudi-common/src/main/java/org/apache/hudi/common/model/MetaFieldsMode.java:
##########
@@ -0,0 +1,191 @@
+/*
+ * 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.config.HoodieConfig;
+import org.apache.hudi.common.table.HoodieTableConfig;
+import org.apache.hudi.common.util.StringUtils;
+
+import java.util.Locale;
+
+/**
+ * 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;
+ }
+
+ /**
Review Comment:
Done in
[`5f397915d1d8`](https://github.com/apache/hudi/pull/19205/commits/5f397915d1d8)
— the block now sits directly above `resolve(String, boolean)`.
--
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]
