hudi-agent commented on code in PR #19205:
URL: https://github.com/apache/hudi/pull/19205#discussion_r3686728626
##########
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:
🤖 nit: `MetaFieldsMode` is referenced with the full package prefix twice
here (`org.apache.hudi.common.model.MetaFieldsMode`). Could you add it to the
import block instead? `HoodieSparkParquetWriter` in the same module already
imports it, so it's available.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
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:
🤖 nit: this Javadoc block (with `@param rawMode`, `@param
legacyPopulateMetaFields`, and `@throws`) documents the two-arg
`resolve(String, boolean)` overload, but it sits immediately above the
*one-arg* `resolve(HoodieConfig)` method — so it's orphaned and IDEs will
attach it to the wrong method. Could you move it to sit directly above
`resolve(String, boolean)` at line ~130?
<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:
##########
@@ -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:
🤖 nit: same inline fully-qualified name as in `HoodieSparkFileWriterFactory`
— could you add `import org.apache.hudi.common.model.MetaFieldsMode;` here too
and use the simple name?
<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/HoodieAvroParquetWriter.java:
##########
@@ -48,39 +51,72 @@ public class HoodieAvroParquetWriter
private final String fileName;
private final String instantTime;
private final TaskContextSupplier taskContextSupplier;
- private final boolean populateMetaFields;
+ private final MetaFieldsMode metaFieldsMode;
private final HoodieAvroWriteSupport writeSupport;
+ /**
+ * @deprecated since 1.3.0, use the {@link MetaFieldsMode} overload.
Retained for existing callers
+ * that only distinguish all-or-nothing meta fields ({@code true} maps to
{@link MetaFieldsMode#ALL},
+ * {@code false} to {@link MetaFieldsMode#NONE}); it cannot express the
selective modes.
+ */
+ @Deprecated
@SuppressWarnings({"unchecked", "rawtypes"})
public HoodieAvroParquetWriter(StoragePath file,
HoodieParquetConfig<HoodieAvroWriteSupport>
parquetConfig,
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,
+ HoodieParquetConfig<HoodieAvroWriteSupport>
parquetConfig,
+ String instantTime,
+ TaskContextSupplier taskContextSupplier,
+ MetaFieldsMode metaFieldsMode) throws
IOException {
super(file, (HoodieParquetConfig) parquetConfig);
this.fileName = file.getName();
this.writeSupport = parquetConfig.getWriteSupport();
this.instantTime = instantTime;
this.taskContextSupplier = taskContextSupplier;
- this.populateMetaFields = populateMetaFields;
+ this.metaFieldsMode = metaFieldsMode == null ? MetaFieldsMode.NONE :
metaFieldsMode;
}
@Override
public void writeAvroWithMetadata(HoodieKey key, IndexedRecord avroRecord)
throws IOException {
- if (populateMetaFields) {
- prepRecordWithMetadata(key, avroRecord, instantTime,
- taskContextSupplier.getPartitionIdSupplier().get(),
getWrittenRecordCount(), fileName);
- super.write(avroRecord);
- writeSupport.add(key.getRecordKey());
- } else {
- super.write(avroRecord);
+ switch (metaFieldsMode) {
+ case ALL:
+ prepRecordWithMetadata(key, avroRecord, instantTime,
+ taskContextSupplier.getPartitionIdSupplier().get(),
getWrittenRecordCount(), fileName);
+ super.write(avroRecord);
+ writeSupport.add(key.getRecordKey());
+ break;
+ case NONE:
+ super.write(avroRecord);
+ break;
+ default:
+ // Selective mode — populate only the opted-in columns. The other meta
columns stay null,
+ // which Parquet stores as definition-level flags (zero data bytes).
Bloom filter /
+ // record-key index population is intentionally skipped — that
requires the record-key
+ // column, which is never populated in selective modes.
+ GenericRecord genericRecord = (GenericRecord) avroRecord;
+ if (metaFieldsMode.isCommitTimePopulated()) {
+ genericRecord.put(HoodieRecord.COMMIT_TIME_METADATA_FIELD,
instantTime);
+ }
+ if (metaFieldsMode.isFileNamePopulated()) {
Review Comment:
🤖 This `isFileNamePopulated()` gate only guards `writeAvroWithMetadata`. The
merge/compaction preserve-metadata copy path goes through
`HoodieWriteMergeHandle.writeToFile(shouldPreserveRecordMetadata=true)`, which
does `record.updateMetaField(FILENAME_META_FIELD_ORD, newFile)` and then
`fileWriter.write(...)` → `writeAvro` (line 117), which writes the record as-is
with no mode gating. So for a `COMMIT_TIME_ONLY` table, copied/unchanged
records during an upsert (and records during compaction) still get
`_hoodie_file_name` populated, which seems to contradict the "other meta
columns stay null" contract. Is that intended?
<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]
