Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import org.apache.hudi.common.model.HoodieRecord;
import org.apache.hudi.common.model.HoodieTableType;
import org.apache.hudi.common.model.HoodieWriteStat;
import org.apache.hudi.common.model.MetaFieldsMode;
import org.apache.hudi.common.model.TableServiceType;
import org.apache.hudi.common.model.WriteOperationType;
import org.apache.hudi.common.schema.HoodieSchema;
Expand Down Expand Up @@ -1548,9 +1549,40 @@ public void validateAgainstTableProperties(HoodieTableConfig tableConfig, Hoodie
// mismatch of table versions.
CommonClientUtils.validateTableVersion(tableConfig, writeConfig);

// Once meta fields are disabled, it cant be re-enabled for a given table.
if (!tableConfig.populateMetaFields() && writeConfig.populateMetaFields()) {
throw new HoodieException(HoodieTableConfig.POPULATE_META_FIELDS.key() + " already disabled for the table. Can't be re-enabled back");
// Meta-field population is physical, so a writer must not claim columns the table does not
// have. Compare the full enum rather than the legacy booleans: those collapse every selective
// mode to false, so a writer claiming COMMIT_TIME_ONLY against a NONE table would slip through
// and advertise commit times that were never written.
//
// Two distinct cases, because writers routinely omit meta-field settings entirely:
//
// - Widening is always rejected. Enabling a column now would leave earlier commits without it,
// and readers cannot tell the two apart.
// - Any disagreement is rejected when the writer *explicitly* sets hoodie.meta.fields.mode.
// That covers narrowing too, e.g. an explicit NONE against a COMMIT_TIME_ONLY table, which
// would write null commit times while the table still advertises COMMIT_TIME_ONLY and make
// incremental queries silently miss those rows.
//
// A writer that never mentions the mode is left alone: resolving to NONE against an ALL table
// is long-standing behavior for callers that build a write config without restating the table's
// settings, and writing fewer meta columns cannot make a reader believe in absent data.
MetaFieldsMode tableMetaFieldsMode = tableConfig.getMetaFieldsMode();
MetaFieldsMode writeMetaFieldsMode = writeConfig.getMetaFieldsMode();
boolean writerStatedMode = writeConfig.contains(HoodieTableConfig.META_FIELDS_MODE)
&& !StringUtils.isNullOrEmpty(writeConfig.getString(HoodieTableConfig.META_FIELDS_MODE));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is cshuo's StreamSync-restart scenario, and I do not think it is fixed. It was closed as resolved by c78dc962, but that commit fixed table-config resolution, not this gate.

Walk it through with table = COMMIT_TIME_ONLY and a writer that sets only hoodie.populate.meta.fields=false:

  • writeMetaFieldsMode resolves to NONE
  • NONE.isWiderThan(COMMIT_TIME_ONLY) is false, so the first branch is skipped
  • writerStatedMode is false, so the second branch is skipped
  • validation passes

The writer then takes the mode from the write config, not the table config (HoodieSparkFileWriterFactory.java:59-60), so it writes base files with null _hoodie_commit_time. The table still advertises COMMIT_TIME_ONLY, so IncrementalRelationV1.scala:92 still admits incremental queries, and the range filter at :295-297 silently drops every one of those rows.

StreamSync is the concrete path: it builds its write config from props only (StreamSync.java:1280-1291) and never merges the table config -- the metaClient != null block at :1297-1309 only touches ordering fields. The +2 lines this PR adds live in initializeEmptyTable, which runs only when the base path does not exist, so a restart against an existing table never re-reads the mode. The Spark datasource is safe only by accident, because HoodieSparkSqlWriter.scala:1102-1106 copies table props into the write params.

Same failure shape as 876a891979a3 [HUDI-3544], where the fix was to detect the config/file disagreement and re-initialize rather than let mixed files accumulate.

The narrowing carve-out only needs to cover ALL -> NONE (the pre-PR behaviour from d5026e9a2485). Suggest tightening to:

} else if (writeMetaFieldsMode != tableMetaFieldsMode
           && (writerStatedMode || tableMetaFieldsMode.isSelective())) {

and adding a TestBaseHoodieWriteClient case: table COMMIT_TIME_ONLY, unstated writer -> must throw.

if (writeMetaFieldsMode.isWiderThan(tableMetaFieldsMode)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is narrower than allowed here? seems not from the check in line 1580

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, narrowing is deliberately allowed, and the code does match the comment. Full matrix, with S = "writer explicitly set hoodie.meta.fields.mode":

  • S = true: passes iff writer mode == table mode; all 20 off-diagonal cells throw (that is the :1580 branch).
  • S = false: the writer mode can only be ALL or NONE, since it is derived from the legacy boolean.
    • ALL: throws for every table mode except ALL (caught by isWiderThan via the record-key clause).
    • NONE: passes against all five table modes.

So :1573 is what allows narrowing, and it is only reached when S is false. Of those NONE cells, ALL and NONE are correct and long-standing. The three selective ones are the bug I flagged on :1572 -- they let a writer silently write null _hoodie_commit_time onto a table that still advertises COMMIT_TIME_ONLY.

Worth noting the relation is not a total order: COMMIT_TIME_ONLY and FILE_NAME_ONLY are each wider than the other, so transitions between them are rejected in both directions. Nothing currently tests that.

throw new HoodieException(String.format(
"%s cannot be widened for an existing table: table is %s but the writer requests %s. Meta "
+ "columns are physical, so enabling one now would leave earlier commits without it. "
+ "Set %s=%s on the writer, or recreate the table to change it.",
HoodieTableConfig.META_FIELDS_MODE.key(), tableMetaFieldsMode, writeMetaFieldsMode,
HoodieTableConfig.META_FIELDS_MODE.key(), tableMetaFieldsMode));
} else if (writerStatedMode && writeMetaFieldsMode != tableMetaFieldsMode) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if writeMetaFieldsMode != tableMetaFieldsMode is true, then writerStatedMode must be true right? so we can eliminate writerStatedMode.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it is load-bearing, and this PR already has the test that proves it: TestBaseHoodieWriteClient.java:129-141 (validateAgainstTablePropertiesAllowsUnstatedWriterToNarrow) -- table ALL, writer sets only withPopulateMetaFields(false), so writeMetaFieldsMode is NONE and differs from the table's ALL while writerStatedMode is false.

That path has always been legal: the pre-PR check was one-directional (!table.populateMetaFields() && write.populateMetaFields()), added in d5026e9a2485 [HUDI-2161]. Dropping the flag would start throwing for every caller that builds a write config without restating the table's meta-field settings.

That said, the flag is currently mis-scoped rather than redundant -- see my comment on :1572. It should gate only ALL tables; for a selective table the disagreement has to be rejected whether or not the writer stated a mode.

throw new HoodieException(String.format(
"%s mismatch: table is %s but the writer explicitly requests %s. Meta columns are physical, "
+ "so the writer must match the table. Set %s=%s on the writer, or recreate the table.",
HoodieTableConfig.META_FIELDS_MODE.key(), tableMetaFieldsMode, writeMetaFieldsMode,
HoodieTableConfig.META_FIELDS_MODE.key(), tableMetaFieldsMode));
}

// Meta fields can be disabled only when either {@code SimpleKeyGenerator}, {@code ComplexKeyGenerator},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import org.apache.hudi.common.model.HoodieRecordMerger;
import org.apache.hudi.common.model.HoodieRecordPayload;
import org.apache.hudi.common.model.HoodieTableType;
import org.apache.hudi.common.model.MetaFieldsMode;
import org.apache.hudi.common.model.WriteConcurrencyMode;
import org.apache.hudi.common.model.WriteOperationType;
import org.apache.hudi.common.table.HoodieTableConfig;
Expand Down Expand Up @@ -1772,8 +1773,39 @@ public int getSmallFileGroupCandidatesLimit() {
return getInt(MERGE_SMALL_FILE_GROUP_CANDIDATES_LIMIT);
}

/**
* @return true when every meta column is populated.
*
* <p>Derived from {@link #getMetaFieldsMode()} so that call sites still written against the
* deprecated {@code hoodie.populate.meta.fields} boolean observe the same answer as the enum:
* only {@link MetaFieldsMode#ALL} populates every meta column.
*/
public boolean populateMetaFields() {
return getBooleanOrDefault(HoodieTableConfig.POPULATE_META_FIELDS);
return getMetaFieldsMode().toLegacyPopulateMetaFields();
}

/**
* @return the {@link MetaFieldsMode} resolved from the write config.
* {@code hoodie.meta.fields.mode} is the source of truth; configs written before that property
* existed fall back to {@link MetaFieldsMode#ALL} or {@link MetaFieldsMode#NONE} based on the
* deprecated {@code hoodie.populate.meta.fields} boolean.
*/
public MetaFieldsMode getMetaFieldsMode() {
return MetaFieldsMode.resolve(this);
}

/**
* @return true when {@code _hoodie_commit_time} is physically populated on every row.
*/
public boolean isCommitTimePopulated() {
return getMetaFieldsMode().isCommitTimePopulated();
}

/**
* @return true when {@code _hoodie_file_name} is physically populated on every row.
*/
public boolean isFileNamePopulated() {
return getMetaFieldsMode().isFileNamePopulated();
}

/**
Expand Down Expand Up @@ -3586,11 +3618,31 @@ public Builder withCanIgnorePostCommitFailures(boolean canIgnorePostCommitFailur
return this;
}

/**
* @deprecated since 1.3.0, use {@link #withMetaFieldsMode(MetaFieldsMode)} instead
* ({@code true} maps to {@link MetaFieldsMode#ALL}, {@code false} to {@link MetaFieldsMode#NONE}).
*/
@Deprecated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may not related with this patch, but setting a table config option though write config API does not look right, the table config option should be immutable once the table is created.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed on the principle, and you're right that it's pre-existing rather than introduced here — withPopulateMetaFields has set HoodieTableConfig.POPULATE_META_FIELDS through the write-config builder since well before this patch (it's on master today at the same line). withMetaFieldsMode follows the established pattern for the same property family, so I'd rather not diverge one config from its sibling inside this PR.

On the immutability concern specifically: the write config is the input to table creation, not a mutation channel for an existing table. Once a table exists, a differing value is rejected rather than applied — BaseHoodieWriteClient.validateAgainstTableProperties now compares the full MetaFieldsMode (strengthened in c266b82 for your other P1), and HoodieWriterUtils.validateTableConfig blocks the datasource path. So the builder can express an intended mode, but it cannot silently change one on disk.

That said, the broader cleanup you're pointing at — write-config builders shouldn't carry table-config setters at all — seems worth doing properly across all of them rather than piecemeal. Happy to file a follow-up JIRA for that sweep if you think it's worth tracking; it would cover the existing POPULATE_META_FIELDS setter too, which is the one with real callers today.

public Builder withPopulateMetaFields(boolean populateMetaFields) {
writeConfig.setValue(HoodieTableConfig.POPULATE_META_FIELDS, Boolean.toString(populateMetaFields));
return this;
}

public Builder withMetaFieldsMode(MetaFieldsMode metaFieldsMode) {
// Leaving the mode unset defers to the deprecated populate.meta.fields boolean. Setting it
// also rewrites that boolean from the mode, so the two can never disagree — a config carrying
// a selective mode alongside populate.meta.fields=true would otherwise create a table whose
// hoodie.properties misleads pre-1.3.0 readers into treating it as ALL.
if (metaFieldsMode == null) {
writeConfig.setValue(HoodieTableConfig.META_FIELDS_MODE, "");
} else {
writeConfig.setValue(HoodieTableConfig.META_FIELDS_MODE, metaFieldsMode.name());
writeConfig.setValue(HoodieTableConfig.POPULATE_META_FIELDS,
Boolean.toString(metaFieldsMode.toLegacyPopulateMetaFields()));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

withMetaFieldsMode re-derives the boolean, but withPopulateMetaFields does not re-derive the mode -- so the invariant this comment claims holds in only one call order:

.withPopulateMetaFields(true).withMetaFieldsMode(COMMIT_TIME_ONLY)
    -> populate.meta.fields = false   (consistent)
.withMetaFieldsMode(COMMIT_TIME_ONLY).withPopulateMetaFields(true)
    -> populate.meta.fields = true    (contradictory)

The second order produces exactly the config 664ff2ec was meant to make impossible: a selective mode sitting next to populate.meta.fields=true. getMetaFieldsMode() still resolves correctly because the mode wins, but any path that copies raw write-config props into hoodie.properties carries the contradiction to disk.

Cheapest fix is to re-derive in build() rather than in the setter, so order stops mattering. Then TestHoodieWriteConfigMetaFieldsMode:122 can assert the raw property in both orders, which it currently skips for the order that does not hold.

}
return this;
}

public Builder withAllowOperationMetadataField(boolean allowOperationMetadataField) {
writeConfig.setValue(ALLOW_OPERATION_METADATA_FIELD, Boolean.toString(allowOperationMetadataField));
return this;
Expand Down Expand Up @@ -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();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] populate.meta.fields=true must not be accepted with NONE or a selective mode unless the legacy flag is rewritten before table creation. The new builder/table-builder paths persist the two properties independently, so a table can be written selectively while hoodie.properties still says hoodie.populate.meta.fields=true. Pre-1.3 readers ignore the new property and then treat the table as ALL; for NONE, an older incremental reader can be allowed to run while every commit time is null and silently return no rows. Please either reject conflicting combinations or always persist the derived legacy value (ALL -> true, every other mode -> false). This also makes TestMetaFieldsMode#populateTrueWithSelectiveModeIsRejected` fail: no exception is thrown.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 664ff2e — you're right, and the forward-compat consequence you describe is the important part.

Whenever an explicit mode is supplied, the legacy boolean is now derived from it rather than taken from the caller (ALL -> true, every other mode -> false), so hoodie.properties can never contradict the mode:

  • HoodieTableMetaClient.TableBuilder — the chokepoint every table-creation path goes through, including HoodieSparkSqlWriter and StreamSync, which both call setPopulateMetaFields(...) and setMetaFieldsModeFromString(...).
  • HoodieWriteConfig.Builder#withMetaFieldsMode — so a write config handed to table creation cannot carry a contradicting boolean either.

With no mode supplied, only the legacy boolean is recorded, so pre-1.3.0 on-disk behavior is unchanged.

I went with "always persist the derived value" rather than "reject the combination" because rejecting would make populate.meta.fields=true + ALL an error, which is a perfectly coherent request, and because the derived write also fixes the case where the boolean is simply absent.

On TestMetaFieldsMode#populateTrueWithSelectiveModeIsRejected: you're right that it was failing, and I should own how that happened. CI flagged it, and I rewrote it to accept the new behavior — I read it as stale rather than as the signal it was. It's now selectiveModeWinsOverLegacyPopulateTrue, and it additionally asserts the persisted boolean is false, which is the invariant that was actually broken. Added noneModePersistsLegacyBooleanAsFalse and allModePersistsLegacyBooleanAsTrue alongside it, plus builder-level coverage of all four combinations in TestHoodieTableMetaClient.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can have a method MetaFieldsMode.isSelective

checkArgument(!(writeConfig.getTableType() == HoodieTableType.MERGE_ON_READ && isSelective),
String.format("%s=%s is currently supported for COPY_ON_WRITE tables only. MoR support is a follow-up. "
+ "For MoR use %s=ALL or %s=NONE.",
HoodieTableConfig.META_FIELDS_MODE.key(), metaFieldsMode,
HoodieTableConfig.META_FIELDS_MODE.key(), HoodieTableConfig.META_FIELDS_MODE.key()));
// Selective meta-field modes are wired only for the Spark writer path in this release. Flink
// RowData / Java-client writers ignore the mode and would silently produce NONE-mode output.
checkArgument(!(engineType != EngineType.SPARK && isSelective),
Comment thread
nsivabalan marked this conversation as resolved.
String.format("%s=%s is currently supported for the Spark writer only. Support for engine=%s is a follow-up. "
+ "Use %s=ALL or %s=NONE.",
HoodieTableConfig.META_FIELDS_MODE.key(), metaFieldsMode, engineType,
HoodieTableConfig.META_FIELDS_MODE.key(), HoodieTableConfig.META_FIELDS_MODE.key()));
}

public HoodieWriteConfig build() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,25 @@

package org.apache.hudi.table.upgrade;

import org.apache.hudi.common.config.ConfigProperty;
import org.apache.hudi.common.engine.HoodieEngineContext;
import org.apache.hudi.common.model.MetaFieldsMode;
import org.apache.hudi.common.table.HoodieTableConfig;
import org.apache.hudi.config.HoodieWriteConfig;

import java.util.Collections;
import java.util.Map;

/**
* Version 10 enables native log format by default for new writes. Existing version 9
* inline log files remain readable by version 10 readers, so there is no table metadata
* rewrite required for the upgrade.
*
* <p>Version 10 also introduced {@code hoodie.meta.fields.mode}. Version 9 tables predate it and
* resolve to {@code ALL} / {@code NONE} from the deprecated {@code hoodie.populate.meta.fields}
* boolean. The upgrade records that derived value explicitly so upgraded tables describe their
* meta-field layout the same way newly created version 10 tables do, rather than depending on the
* legacy fallback. Behavior is unchanged either way — this only makes the on-disk state explicit.
*/
public class NineToTenUpgradeHandler implements UpgradeHandler {

Expand All @@ -34,6 +46,12 @@ public UpgradeDowngrade.TableConfigChangeSet upgrade(
HoodieEngineContext context,
String instantTime,
SupportsUpgradeDowngrade upgradeDowngradeHelper) {
return new UpgradeDowngrade.TableConfigChangeSet();
HoodieTableConfig tableConfig =
upgradeDowngradeHelper.getTable(config, context).getMetaClient().getTableConfig();
// Resolves from the legacy boolean for a version 9 table, since the mode property is absent.
MetaFieldsMode metaFieldsMode = tableConfig.getMetaFieldsMode();
Map<ConfigProperty, String> propertiesToUpdate = Collections.singletonMap(
HoodieTableConfig.META_FIELDS_MODE, metaFieldsMode.name());
return new UpgradeDowngrade.TableConfigChangeSet(propertiesToUpdate, Collections.emptySet());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the legacy "populate meta fields" property should be removed?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would keep it, and I think this one deserves an explicit comment saying why, because the symmetry argument points the other way.

EightToNineUpgradeHandler does remove the legacy property (:171-172, :240-241), so the convention says remove it here too. The difference is the default: PAYLOAD_CLASS_NAME and PRECOMBINE_FIELD have no default, but POPULATE_META_FIELDS defaults to true (HoodieTableConfig.java:338-341).

So if the upgrade removed the boolean and the table were later downgraded, a v9 reader would find neither property and resolve to ALL -- claiming meta columns on files that do not have them. That is exactly the widening this PR bans everywhere else.

Suggest keeping it and noting the default-value exception in the class javadoc, so the deviation reads as deliberate rather than an oversight.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if the upgrade removed the boolean and the table were later downgraded, a v9 reader would find neither property and resolve to ALL

Didn't get it, for upgrade, the POPULATE_META_FIELDS would be translated into ALL or NONE mode, and for downgrade the ALL or NONE would be translated back into POPULATE_META_FIELDS true or false right? For default value that is not declared explicitly, we can skip the translation since both options have the same default value, for explicit options, let's just keep one of them to avoid ambiguties.

@voonhous voonhous Jul 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a scenario for upgrade + downgrade round trip.

State A -- v9 table today (created pre-1.3 with virtual keys):

hoodie.properties:

hoodie.table.version=9
hoodie.populate.meta.fields=false

Data files on disk have no meta columns. Readers resolve populateMetaFields() == false and use the user's key field.

State B -- after 9 -> 10

if the upgrade removed the legacy property (the symmetry that EightToNineUpgradeHandler follows for PAYLOAD_CLASS_NAME at :171-172 and PRECOMBINE_FIELD at :240-241):

hoodie.properties:

hoodie.table.version=10
hoodie.meta.fields.mode=NONE

It is still correct -- at v10 the mode is the source of truth.

State C -- after 10 -> 9 downgrade.

TenToNineDowngradeHandler:75 deletes META_FIELDS_MODE, and it has to: v9 cannot interpret it.

hoodie.properties:

hoodie.table.version=9

Neither property is present now.

State D -- a v9 reader opens it

HoodieTableConfig.getMetaFieldsMode() (:1281) falls back to legacyPopulateMetaFields() -> getStringOrDefault(POPULATE_META_FIELDS) -> #defaultValue(true) (:338-341) -> ALL.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During downgrade, the hoodie.populate.meta.fields should be added back correctly with value inferred from the META_FIELDS_MODE so that the reader can still work as before.

We should fix both the upgrade/downgrade handler to translate the options and add/remove the option that does not need

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reached concensus offline with @voonhous that my suggestions works.

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,61 @@

package org.apache.hudi.table.upgrade;

import org.apache.hudi.common.config.ConfigProperty;
import org.apache.hudi.common.engine.HoodieEngineContext;
import org.apache.hudi.common.model.MetaFieldsMode;
import org.apache.hudi.common.table.HoodieTableConfig;
import org.apache.hudi.config.HoodieWriteConfig;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

/**
* Version 10 writes native log files by default. Downgrading to version 9 requires
* full compaction of native data/delete logs before the downgrade completes.
*
* <p>Version 10 also introduced {@code hoodie.meta.fields.mode}. Version 9 does not understand it,
* so the property is dropped here while {@code hoodie.populate.meta.fields} is left exactly as it
* stands — {@code ALL} and {@code NONE} tables round-trip unchanged because those are precisely the
* two states the legacy boolean can express. Selective modes cannot be expressed in version 9, so
* the table degrades to what its legacy boolean says (which is {@code false}, i.e. NONE) and we warn.
*/
public class TenToNineDowngradeHandler implements DowngradeHandler {

private static final Logger LOG = LoggerFactory.getLogger(TenToNineDowngradeHandler.class);

@Override
public UpgradeDowngrade.TableConfigChangeSet downgrade(
HoodieWriteConfig config,
HoodieEngineContext context,
String instantTime,
SupportsUpgradeDowngrade upgradeDowngradeHelper) {
Set<ConfigProperty> propertiesToDelete = new HashSet<>();
propertiesToDelete.add(HoodieTableConfig.TABLE_STORAGE_LAYOUT);

// The warning is best-effort: dropping the property is what matters, and the helper is not
// always available (some callers drive the change set directly).
MetaFieldsMode metaFieldsMode = upgradeDowngradeHelper == null
? MetaFieldsMode.ALL
: upgradeDowngradeHelper.getTable(config, context).getMetaClient().getTableConfig().getMetaFieldsMode();
if (metaFieldsMode != MetaFieldsMode.ALL && metaFieldsMode != MetaFieldsMode.NONE) {
LOG.warn("Table is using {}={}, which table version 9 cannot express. The property is being "
+ "removed and the table will behave as {}=false (no meta columns) to version 9 readers. "
+ "Already-written files keep their populated meta columns, but incremental queries that "
+ "relied on {} will stop returning rows. Recreate the table if you need that behavior back.",
HoodieTableConfig.META_FIELDS_MODE.key(), metaFieldsMode,
HoodieTableConfig.POPULATE_META_FIELDS.key(), metaFieldsMode);
}
// hoodie.populate.meta.fields is deliberately left untouched: whatever the table recorded before
// the downgrade stays, so ALL and NONE tables are bit-identical afterwards.
propertiesToDelete.add(HoodieTableConfig.META_FIELDS_MODE);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks the symmetry convention this package already established, and the result is lossy.

The precedent, from b60d38c40fa1 [HUDI-8401]:

  • upgrade adds the new property and removes the legacy one -- EightToNineUpgradeHandler.java:171-172 and :240-241
  • downgrade restores the legacy one from the new one and removes the new one -- NineToEightDowngradeHandler.java:116-117 and :152-153

Here the downgrade deletes hoodie.meta.fields.mode and deliberately writes nothing back. Two consequences:

  1. If a table ever carries the mode without the boolean, POPULATE_META_FIELDS defaults to true, so the table downgrades to ALL -- Hudi then believes _hoodie_record_key is populated on files where it is physically null.
  2. A selective mode is unrecoverable. Downgrade drops it, re-upgrade derives NONE from the boolean, and isWiderThan now rejects any writer trying to restore COMMIT_TIME_ONLY as a widening. No other handler pair in this package is one-way like that.

Suggest adding the derived boolean here, mirroring NineToEightDowngradeHandler.java:117:

propertiesToUpdate.put(HoodieTableConfig.POPULATE_META_FIELDS.key(),
    String.valueOf(metaFieldsMode.toLegacyPopulateMetaFields()));

For ALL/NONE that is a no-op, so nothing regresses; it just closes the hole. (This also answers your open question at :76 -- yes, infer and add it.)

Separately: a selective mode is silently destroyed here with only a LOG.warn. Consider throwing unless the caller opts in, since the operator gets no acknowledgement today.


return new UpgradeDowngrade.TableConfigChangeSet(
Collections.emptyMap(),
Collections.singleton(HoodieTableConfig.TABLE_STORAGE_LAYOUT));
propertiesToDelete);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the legacy "populate meta fields" property should be inferred and added here?

@voonhous voonhous Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 Details on :72.

The precedent is NineToEightDowngradeHandler.java:116-117 and :152-153: downgrade restores the legacy property from the new one, then removes the new one. Here the mode is deleted and nothing is written back.

For ALL/NONE writing the derived boolean is a no-op, so it costs nothing. What it fixes is the case where a table carries the mode without the boolean -- POPULATE_META_FIELDS then falls back to its true default and the table downgrades to ALL, i.e. Hudi believes _hoodie_record_key is populated on files where it is null.

propertiesToUpdate.put(HoodieTableConfig.POPULATE_META_FIELDS.key(),
    String.valueOf(metaFieldsMode.toLegacyPopulateMetaFields()));

Separate but related: selective modes are destroyed here with only a LOG.warn, and they cannot be restored afterwards, because a re-upgrade derives NONE and isWiderThan then rejects setting the mode back. No other handler pair in this package is one-way like that -- worth either throwing or documenting it as intentional.

}
}
Loading
Loading