voonhous commented on code in PR #19205:
URL: https://github.com/apache/hudi/pull/19205#discussion_r3665544190
##########
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:
+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.
```java
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.
--
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]
