hudi-agent commented on code in PR #19378: URL: https://github.com/apache/hudi/pull/19378#discussion_r3687152264
########## 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 Review Comment: 🤖 nit: the `@param rawMode` / `@param legacyPopulateMetaFields` / `@throws` Javadoc block is sitting directly above `resolve(HoodieConfig config)` rather than above `resolve(String, boolean)` — could you move it so it attaches to the right method? As written, Javadoc tooling binds it to the single-arg overload (which has none of those params), and the two-arg method gets no docs at all. <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub> ########## hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieWriterUtils.scala: ########## @@ -352,6 +352,23 @@ object HoodieWriterUtils { diffConfigs.append(s"${HoodieTableConfig.RECORD_MERGE_STRATEGY_ID}:\t$mergeStrategyId\tnull\n") } } + + // hoodie.meta.fields.mode is a physical-storage decision baked into files at write time. + // Changing it at runtime would silently produce mixed-mode files whose incremental / file + // pruning behavior differs between old and new commits. The default loop above only flags + // the mismatch when the on-disk value is non-null, so an older table with the property + // absent from hoodie.properties would let a null → non-empty transition slip through + // (silent-drop risk on pre-enablement commits). Guard the null → selective-mode case + // explicitly here. Set it only at table creation, via the hudi-cli, or during table upgrade; + // otherwise the only way to change it is to recreate the table. + val paramsMetaFieldsMode = params.getOrElse(HoodieTableConfig.META_FIELDS_MODE.key(), "") + val onDiskMetaFieldsMode = tableConfig.getString(HoodieTableConfig.META_FIELDS_MODE) + if (paramsMetaFieldsMode.nonEmpty && (onDiskMetaFieldsMode == null || onDiskMetaFieldsMode.isEmpty)) { Review Comment: 🤖 This fires whenever `params` sets a non-empty mode and the on-disk value is null, which also catches the no-op cases: `mode=ALL` on an existing default table (on-disk `populate.meta.fields=true` ⇒ already ALL) and `mode=NONE` on an existing `populate=false` table. Should it compare against the mode the legacy boolean already implies, so a matching (no-op) mode isn't rejected as an immutable change? A platform that stamps `hoodie.meta.fields.mode=ALL` on every write would otherwise start failing on all pre-feature tables. @yihua does this match the intended backward-compat behavior? <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: ########## @@ -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: could you add `import org.apache.hudi.common.model.MetaFieldsMode;` and use the short name throughout? The fully-qualified `org.apache.hudi.common.model.MetaFieldsMode` appears inline on the Parquet, Lance, and Vortex paths and makes those expressions harder to scan than a one-word name would. <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]
