nsivabalan commented on code in PR #19205:
URL: https://github.com/apache/hudi/pull/19205#discussion_r3707958142
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/NineToTenUpgradeHandler.java:
##########
@@ -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());
Review Comment:
Implemented in
[`69a0906419d8`](https://github.com/apache/hudi/pull/19205/commits/69a0906419d8)
— this is the consensus you two reached on 07-29: translate on both edges,
keep both properties in agreement.
**Downgrade (10 → 9)** now writes the legacy boolean back from the mode
(`ALL -> true`, every other mode `-> false`) and deletes the mode, mirroring
`NineToEightDowngradeHandler:116-117`. For `ALL`/`NONE` that is a no-op
restatement, so nothing regresses; what it closes is the case @voonhous
described — a table carrying the mode without the boolean, where
`POPULATE_META_FIELDS` 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.
**Upgrade (9 → 10)** keeps the boolean rather than deleting it, which is
where this deviates from `EightToNineUpgradeHandler`. That deviation is now
spelled out in the class javadoc: the properties precedent removes
(`PAYLOAD_CLASS_NAME`, `PRECOMBINE_FIELD`) have no default, so deleting them is
unambiguous; `POPULATE_META_FIELDS` defaults to `true`, so deleting it is
precisely what creates the silent-widening case above.
One more fix in the same commit, not raised but adjacent: when no
`SupportsUpgradeDowngrade` helper was passed the handler assumed `ALL`. It now
leaves the boolean untouched instead of deriving a value from a guess — writing
a derived boolean from an assumed mode is the same failure shape in miniature.
**On the one-way concern** (@voonhous, `:76`): a selective mode is still
unrecoverable across a round trip, and I left that as a warning rather than a
hard failure. The warning now says so explicitly — that re-upgrading resolves
to `NONE` and widening back is rejected — so the operator is told the downgrade
is lossy before it happens. Happy to make it throw behind an opt-in flag if you
would rather it not be silent-by-default; that felt like it belonged with the
CLI work in #19206 rather than here.
**Tests.** `TestTenToNineDowngradeHandler` previously passed `null` for the
helper, so the handler short-circuited to `ALL` and the whole selective branch
was dead — good catch. It now uses the `helperFor(...)` mock factory, covers
all five modes, the no-helper case, and an explicit round-trip assertion. I
verified it is not vacuous: reverting the write-back fails 7 of the 10 tests.
`TestNineToTenUpgradeHandler`'s redundant second case now asserts
`POPULATE_META_FIELDS` appears in neither the update nor the delete set, which
is what its name claimed.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/TenToNineDowngradeHandler.java:
##########
@@ -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);
+
return new UpgradeDowngrade.TableConfigChangeSet(
Collections.emptyMap(),
- Collections.singleton(HoodieTableConfig.TABLE_STORAGE_LAYOUT));
+ propertiesToDelete);
Review Comment:
Implemented in
[`69a0906419d8`](https://github.com/apache/hudi/pull/19205/commits/69a0906419d8)
— this is the consensus you two reached on 07-29: translate on both edges,
keep both properties in agreement.
**Downgrade (10 → 9)** now writes the legacy boolean back from the mode
(`ALL -> true`, every other mode `-> false`) and deletes the mode, mirroring
`NineToEightDowngradeHandler:116-117`. For `ALL`/`NONE` that is a no-op
restatement, so nothing regresses; what it closes is the case @voonhous
described — a table carrying the mode without the boolean, where
`POPULATE_META_FIELDS` 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.
**Upgrade (9 → 10)** keeps the boolean rather than deleting it, which is
where this deviates from `EightToNineUpgradeHandler`. That deviation is now
spelled out in the class javadoc: the properties precedent removes
(`PAYLOAD_CLASS_NAME`, `PRECOMBINE_FIELD`) have no default, so deleting them is
unambiguous; `POPULATE_META_FIELDS` defaults to `true`, so deleting it is
precisely what creates the silent-widening case above.
One more fix in the same commit, not raised but adjacent: when no
`SupportsUpgradeDowngrade` helper was passed the handler assumed `ALL`. It now
leaves the boolean untouched instead of deriving a value from a guess — writing
a derived boolean from an assumed mode is the same failure shape in miniature.
**On the one-way concern** (@voonhous, `:76`): a selective mode is still
unrecoverable across a round trip, and I left that as a warning rather than a
hard failure. The warning now says so explicitly — that re-upgrading resolves
to `NONE` and widening back is rejected — so the operator is told the downgrade
is lossy before it happens. Happy to make it throw behind an opt-in flag if you
would rather it not be silent-by-default; that felt like it belonged with the
CLI work in #19206 rather than here.
**Tests.** `TestTenToNineDowngradeHandler` previously passed `null` for the
helper, so the handler short-circuited to `ALL` and the whole selective branch
was dead — good catch. It now uses the `helperFor(...)` mock factory, covers
all five modes, the no-helper case, and an explicit round-trip assertion. I
verified it is not vacuous: reverting the write-back fails 7 of the 10 tests.
`TestNineToTenUpgradeHandler`'s redundant second case now asserts
`POPULATE_META_FIELDS` appears in neither the update nor the delete set, which
is what its name claimed.
##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/table/upgrade/TestTenToNineDowngradeHandler.java:
##########
@@ -24,18 +24,25 @@
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
class TestTenToNineDowngradeHandler {
@Test
- void testDowngradeRemovesStorageLayoutOnly() {
+ void testDowngradeRemovesStorageLayoutAndMetaFieldsMode() {
UpgradeDowngrade.TableConfigChangeSet changeSet =
new TenToNineDowngradeHandler().downgrade(null, null, null, null);
assertTrue(changeSet.propertiesToUpdate().isEmpty());
- assertEquals(1, changeSet.propertiesToDelete().size());
+ assertEquals(2, changeSet.propertiesToDelete().size());
assertTrue(changeSet.propertiesToDelete().contains(HoodieTableConfig.TABLE_STORAGE_LAYOUT));
+ // Version 9 does not understand hoodie.meta.fields.mode, so it is
dropped...
+
assertTrue(changeSet.propertiesToDelete().contains(HoodieTableConfig.META_FIELDS_MODE));
+ // ...while hoodie.populate.meta.fields is deliberately left in place, so
ALL and NONE tables
+ // round-trip unchanged — those are exactly the two states the legacy
boolean can express.
Review Comment:
Implemented in
[`69a0906419d8`](https://github.com/apache/hudi/pull/19205/commits/69a0906419d8)
— this is the consensus you two reached on 07-29: translate on both edges,
keep both properties in agreement.
**Downgrade (10 → 9)** now writes the legacy boolean back from the mode
(`ALL -> true`, every other mode `-> false`) and deletes the mode, mirroring
`NineToEightDowngradeHandler:116-117`. For `ALL`/`NONE` that is a no-op
restatement, so nothing regresses; what it closes is the case @voonhous
described — a table carrying the mode without the boolean, where
`POPULATE_META_FIELDS` 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.
**Upgrade (9 → 10)** keeps the boolean rather than deleting it, which is
where this deviates from `EightToNineUpgradeHandler`. That deviation is now
spelled out in the class javadoc: the properties precedent removes
(`PAYLOAD_CLASS_NAME`, `PRECOMBINE_FIELD`) have no default, so deleting them is
unambiguous; `POPULATE_META_FIELDS` defaults to `true`, so deleting it is
precisely what creates the silent-widening case above.
One more fix in the same commit, not raised but adjacent: when no
`SupportsUpgradeDowngrade` helper was passed the handler assumed `ALL`. It now
leaves the boolean untouched instead of deriving a value from a guess — writing
a derived boolean from an assumed mode is the same failure shape in miniature.
**On the one-way concern** (@voonhous, `:76`): a selective mode is still
unrecoverable across a round trip, and I left that as a warning rather than a
hard failure. The warning now says so explicitly — that re-upgrading resolves
to `NONE` and widening back is rejected — so the operator is told the downgrade
is lossy before it happens. Happy to make it throw behind an opt-in flag if you
would rather it not be silent-by-default; that felt like it belonged with the
CLI work in #19206 rather than here.
**Tests.** `TestTenToNineDowngradeHandler` previously passed `null` for the
helper, so the handler short-circuited to `ALL` and the whole selective branch
was dead — good catch. It now uses the `helperFor(...)` mock factory, covers
all five modes, the no-helper case, and an explicit round-trip assertion. I
verified it is not vacuous: reverting the write-back fails 7 of the 10 tests.
`TestNineToTenUpgradeHandler`'s redundant second case now asserts
`POPULATE_META_FIELDS` appears in neither the update nor the delete set, which
is what its name claimed.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/TenToNineDowngradeHandler.java:
##########
@@ -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);
Review Comment:
Implemented in
[`69a0906419d8`](https://github.com/apache/hudi/pull/19205/commits/69a0906419d8)
— this is the consensus you two reached on 07-29: translate on both edges,
keep both properties in agreement.
**Downgrade (10 → 9)** now writes the legacy boolean back from the mode
(`ALL -> true`, every other mode `-> false`) and deletes the mode, mirroring
`NineToEightDowngradeHandler:116-117`. For `ALL`/`NONE` that is a no-op
restatement, so nothing regresses; what it closes is the case @voonhous
described — a table carrying the mode without the boolean, where
`POPULATE_META_FIELDS` 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.
**Upgrade (9 → 10)** keeps the boolean rather than deleting it, which is
where this deviates from `EightToNineUpgradeHandler`. That deviation is now
spelled out in the class javadoc: the properties precedent removes
(`PAYLOAD_CLASS_NAME`, `PRECOMBINE_FIELD`) have no default, so deleting them is
unambiguous; `POPULATE_META_FIELDS` defaults to `true`, so deleting it is
precisely what creates the silent-widening case above.
One more fix in the same commit, not raised but adjacent: when no
`SupportsUpgradeDowngrade` helper was passed the handler assumed `ALL`. It now
leaves the boolean untouched instead of deriving a value from a guess — writing
a derived boolean from an assumed mode is the same failure shape in miniature.
**On the one-way concern** (@voonhous, `:76`): a selective mode is still
unrecoverable across a round trip, and I left that as a warning rather than a
hard failure. The warning now says so explicitly — that re-upgrading resolves
to `NONE` and widening back is rejected — so the operator is told the downgrade
is lossy before it happens. Happy to make it throw behind an opt-in flag if you
would rather it not be silent-by-default; that felt like it belonged with the
CLI work in #19206 rather than here.
**Tests.** `TestTenToNineDowngradeHandler` previously passed `null` for the
helper, so the handler short-circuited to `ALL` and the whole selective branch
was dead — good catch. It now uses the `helperFor(...)` mock factory, covers
all five modes, the no-helper case, and an explicit round-trip assertion. I
verified it is not vacuous: reverting the write-back fails 7 of the 10 tests.
`TestNineToTenUpgradeHandler`'s redundant second case now asserts
`POPULATE_META_FIELDS` appears in neither the update nor the delete set, which
is what its name claimed.
##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/config/TestHoodieWriteConfigMetaFieldsMode.java:
##########
@@ -0,0 +1,189 @@
+/*
+ * 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.config;
+
+import org.apache.hudi.common.model.HoodieTableType;
+import org.apache.hudi.common.model.MetaFieldsMode;
+import org.apache.hudi.common.table.HoodieTableConfig;
+
+import org.junit.jupiter.api.Test;
+
+import java.util.Properties;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Validates the writer-side accessors and validation guards for the
meta-field-population modes
+ * on {@link HoodieWriteConfig}. Companion test for the {@link
HoodieTableConfig} accessors lives
+ * in {@code TestHoodieMetaFieldsMode}; this test covers the writer-builder
surface and the
+ * cross-flag validation that runs at {@code build()} time.
+ */
+class TestHoodieWriteConfigMetaFieldsMode {
Review Comment:
Fixed in
[`69a0906419d8`](https://github.com/apache/hudi/pull/19205/commits/69a0906419d8)
— you were right that the `null` helper made the entire selective branch dead
code.
The class now uses the `helperFor(...)` mock factory (copied from
`TestNineToTenUpgradeHandler` as you suggested) and covers all five modes plus
the no-helper path. I confirmed the new cases actually discriminate: reverting
the downgrade write-back fails 7 of the 10 tests, so they are not vacuous.
##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/table/upgrade/TestNineToTenUpgradeHandler.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.table.upgrade;
+
+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.common.table.HoodieTableMetaClient;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.table.HoodieTable;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Version 9 tables predate {@code hoodie.meta.fields.mode}, so the upgrade
records the value
+ * derived from the deprecated {@code hoodie.populate.meta.fields} boolean.
This makes an upgraded
+ * table describe its meta-field layout the same way a freshly created version
10 table does,
+ * instead of relying on the legacy fallback at every read.
+ */
+class TestNineToTenUpgradeHandler {
+
+ private static SupportsUpgradeDowngrade helperFor(MetaFieldsMode
resolvedMode) {
+ HoodieTable table = mock(HoodieTable.class, RETURNS_DEEP_STUBS);
+ HoodieTableMetaClient metaClient = mock(HoodieTableMetaClient.class,
RETURNS_DEEP_STUBS);
+ HoodieTableConfig tableConfig = mock(HoodieTableConfig.class);
+ when(tableConfig.getMetaFieldsMode()).thenReturn(resolvedMode);
+ when(metaClient.getTableConfig()).thenReturn(tableConfig);
+ when(table.getMetaClient()).thenReturn(metaClient);
+
+ SupportsUpgradeDowngrade helper = mock(SupportsUpgradeDowngrade.class);
+
when(helper.getTable(org.mockito.ArgumentMatchers.any(HoodieWriteConfig.class),
+
org.mockito.ArgumentMatchers.any(HoodieEngineContext.class))).thenReturn(table);
+ return helper;
+ }
+
+ @ParameterizedTest
+ @CsvSource({"ALL", "NONE"})
+ void upgradeRecordsTheModeDerivedFromTheLegacyBoolean(String modeName) {
+ MetaFieldsMode expected = MetaFieldsMode.valueOf(modeName);
+ UpgradeDowngrade.TableConfigChangeSet changeSet = new
NineToTenUpgradeHandler().upgrade(
+ mock(HoodieWriteConfig.class), mock(HoodieEngineContext.class), "001",
helperFor(expected));
+
+ assertTrue(changeSet.propertiesToDelete().isEmpty());
+ assertEquals(1, changeSet.propertiesToUpdate().size());
+ assertEquals(expected.name(),
+
changeSet.propertiesToUpdate().get(HoodieTableConfig.META_FIELDS_MODE));
+ }
+
+ @Test
+ void upgradeLeavesTheLegacyBooleanAlone() {
Review Comment:
Fixed in
[`69a0906419d8`](https://github.com/apache/hudi/pull/19205/commits/69a0906419d8)
— took the second option: it now asserts `POPULATE_META_FIELDS` appears in
neither `propertiesToUpdate()` nor `propertiesToDelete()`, which is what the
name claimed and what the parameterized test above genuinely does not cover.
That assertion is also load-bearing now rather than cosmetic: keeping the
boolean on upgrade is what makes the round trip lossless once the downgrade
writes it back, so a regression that started deleting it here would be caught.
--
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]
