voonhous commented on code in PR #19206:
URL: https://github.com/apache/hudi/pull/19206#discussion_r3665334318
##########
hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestTableCommand.java:
##########
@@ -304,4 +305,140 @@ private String getFileContent(String fileToReadStr)
throws IOException {
fis.close();
return fromUTF8Bytes(data);
}
+
+ //
---------------------------------------------------------------------------
+ // set-meta-fields-mode
+ //
---------------------------------------------------------------------------
+
+ @Test
+ public void testSetMetaFieldsModeOnFreshTableToCommitTimeOnly() {
+ assertTrue(prepareTable());
+ // Default table is ALL — no commits yet, so the safety check must let
this through.
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_ONLY");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ // Rendered diff must surface the changed properties so the operator can
confirm the write.
+ String rendered = result.toString();
+ assertTrue(rendered.contains(HoodieTableConfig.POPULATE_META_FIELDS.key()),
+ "expected rendered diff to mention populate.meta.fields, got: " +
rendered);
+ assertTrue(rendered.contains(HoodieTableConfig.META_FIELDS_MODE.key()),
+ "expected rendered diff to mention meta.fields.mode, got: " +
rendered);
+ HoodieTableMetaClient client = HoodieCLI.getTableMetaClient();
+ assertEquals(MetaFieldsMode.COMMIT_TIME_ONLY,
client.getTableConfig().getMetaFieldsMode());
+ assertFalse(client.getTableConfig().populateMetaFields());
+ }
+
+ @Test
+ public void testSetMetaFieldsModeOnFreshTableToFileNameOnly() {
+ assertTrue(prepareTable());
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode FILE_NAME_ONLY");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ assertEquals(MetaFieldsMode.FILE_NAME_ONLY,
+ HoodieCLI.getTableMetaClient().getTableConfig().getMetaFieldsMode());
+ }
+
+ @Test
+ public void testSetMetaFieldsModeOnFreshTableToCombinedMode() {
+ assertTrue(prepareTable());
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_AND_FILE_NAME");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ assertEquals(MetaFieldsMode.COMMIT_TIME_AND_FILE_NAME,
+ HoodieCLI.getTableMetaClient().getTableConfig().getMetaFieldsMode());
+ }
+
+ @Test
+ public void testSetMetaFieldsModeOnFreshTableToNone() {
+ assertTrue(prepareTable());
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode NONE");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ HoodieTableMetaClient client = HoodieCLI.getTableMetaClient();
+ assertEquals(MetaFieldsMode.NONE,
client.getTableConfig().getMetaFieldsMode());
+ assertFalse(client.getTableConfig().populateMetaFields());
+ }
+
+ @Test
+ public void testSetMetaFieldsModeToAllClearsProperty() throws IOException {
+ assertTrue(prepareTable());
+ // First move to a selective mode so we have something to clear.
+ shell.evaluate(() -> "table set-meta-fields-mode --target-mode
COMMIT_TIME_ONLY");
Review Comment:
The result here is discarded. If this transition ever fails the table stays
`ALL`, the `--target-mode ALL` on the next line hits the no-op early return
(`TableCommand.java:292`), and all four assertions below still pass -- a
freshly created table carries neither `populate.meta.fields` nor
`meta.fields.mode`, so `getMetaFieldsMode()` is `ALL`, `populateMetaFields()`
is `true`, and the key is absent.
Wrap this in `assertTrue(ShellEvaluationResultUtil.isSuccess(...))` and
assert the property **is present** before clearing it, so the "was there -> now
gone" transition is actually proven.
##########
hudi-cli/src/main/java/org/apache/hudi/cli/commands/TableCommand.java:
##########
@@ -256,6 +258,82 @@ public String deleteTableConfig(
return renderOldNewProps(newProps, oldProps);
}
+ @ShellMethod(key = "table set-meta-fields-mode",
+ value = "Set hoodie.meta.fields.mode on an existing table. Refuses to
change the mode on a "
Review Comment:
Accuracy point on "the sanctioned way": `table update-configs` sits 30 lines
above and will set `hoodie.meta.fields.mode` from a props file with zero
validation (`:228-244`) -- atomically, in a single modify cycle. `table
delete-configs` and `repair overwrite-hoodie-props` are equally open.
Either move the transition check somewhere all writers go through, or have
`update-configs` / `delete-configs` refuse this key and point here. Otherwise
worth softening the claim in the PR description.
##########
hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestTableCommand.java:
##########
@@ -304,4 +305,140 @@ private String getFileContent(String fileToReadStr)
throws IOException {
fis.close();
return fromUTF8Bytes(data);
}
+
+ //
---------------------------------------------------------------------------
+ // set-meta-fields-mode
+ //
---------------------------------------------------------------------------
Review Comment:
Heads up before these are treated as the verification story: **`hudi-cli`
tests do not run in CI.** The module appears only as an exclusion --
`azure-pipelines-20230430.yml:46,62` and `.github/workflows/bot.yml:242,244` --
and the install steps carry `-DskipTests` (`:109`). No job targets it.
Quick way to confirm: `testDefaultCreate:144` asserts timeline layout
version 1, but `HoodieTableVersion.current()` is `TEN`, which maps to
`LAYOUT_VERSION_2`. That assertion would be red if the suite ran.
So the green Azure run says nothing about these 9 tests. Suggest either
adding `hudi-cli` to the CI module list in a companion change, or putting the
load-bearing coverage somewhere CI actually exercises.
##########
hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestTableCommand.java:
##########
@@ -304,4 +305,140 @@ private String getFileContent(String fileToReadStr)
throws IOException {
fis.close();
return fromUTF8Bytes(data);
}
+
+ //
---------------------------------------------------------------------------
+ // set-meta-fields-mode
+ //
---------------------------------------------------------------------------
+
+ @Test
+ public void testSetMetaFieldsModeOnFreshTableToCommitTimeOnly() {
+ assertTrue(prepareTable());
+ // Default table is ALL — no commits yet, so the safety check must let
this through.
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_ONLY");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ // Rendered diff must surface the changed properties so the operator can
confirm the write.
+ String rendered = result.toString();
+ assertTrue(rendered.contains(HoodieTableConfig.POPULATE_META_FIELDS.key()),
+ "expected rendered diff to mention populate.meta.fields, got: " +
rendered);
+ assertTrue(rendered.contains(HoodieTableConfig.META_FIELDS_MODE.key()),
+ "expected rendered diff to mention meta.fields.mode, got: " +
rendered);
+ HoodieTableMetaClient client = HoodieCLI.getTableMetaClient();
+ assertEquals(MetaFieldsMode.COMMIT_TIME_ONLY,
client.getTableConfig().getMetaFieldsMode());
+ assertFalse(client.getTableConfig().populateMetaFields());
+ }
+
+ @Test
+ public void testSetMetaFieldsModeOnFreshTableToFileNameOnly() {
+ assertTrue(prepareTable());
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode FILE_NAME_ONLY");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ assertEquals(MetaFieldsMode.FILE_NAME_ONLY,
+ HoodieCLI.getTableMetaClient().getTableConfig().getMetaFieldsMode());
+ }
+
+ @Test
+ public void testSetMetaFieldsModeOnFreshTableToCombinedMode() {
+ assertTrue(prepareTable());
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_AND_FILE_NAME");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ assertEquals(MetaFieldsMode.COMMIT_TIME_AND_FILE_NAME,
+ HoodieCLI.getTableMetaClient().getTableConfig().getMetaFieldsMode());
+ }
+
+ @Test
+ public void testSetMetaFieldsModeOnFreshTableToNone() {
+ assertTrue(prepareTable());
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode NONE");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ HoodieTableMetaClient client = HoodieCLI.getTableMetaClient();
+ assertEquals(MetaFieldsMode.NONE,
client.getTableConfig().getMetaFieldsMode());
+ assertFalse(client.getTableConfig().populateMetaFields());
+ }
+
+ @Test
+ public void testSetMetaFieldsModeToAllClearsProperty() throws IOException {
+ assertTrue(prepareTable());
+ // First move to a selective mode so we have something to clear.
+ shell.evaluate(() -> "table set-meta-fields-mode --target-mode
COMMIT_TIME_ONLY");
+ // Then set back to ALL.
+ Object result = shell.evaluate(() -> "table set-meta-fields-mode
--target-mode ALL");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ HoodieTableMetaClient client = HoodieCLI.getTableMetaClient();
+ assertEquals(MetaFieldsMode.ALL,
client.getTableConfig().getMetaFieldsMode());
+ assertTrue(client.getTableConfig().populateMetaFields());
+ // The mode property should be cleared (ALL is implicit).
+
assertFalse(client.getTableConfig().getProps().containsKey(HoodieTableConfig.META_FIELDS_MODE.key()));
+ }
+
+ @Test
+ public void testSetMetaFieldsModeNoOpWhenAlreadyInTargetMode() {
+ assertTrue(prepareTable());
+ Object first = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_ONLY");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(first));
+ // Second call — same target — should be a no-op message.
+ Object second = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_ONLY");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(second));
+ assertTrue(second.toString().contains("already in COMMIT_TIME_ONLY"),
+ "expected no-op message, got: " + second);
+ }
+
+ @Test
+ public void testSetMetaFieldsModeRejectsUnknownValue() {
+ assertTrue(prepareTable());
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode BOGUS_MODE");
+ // Shell evaluate returns the exception object on failure.
+ assertFalse(ShellEvaluationResultUtil.isSuccess(result));
+ assertTrue(result.toString().contains("BOGUS_MODE"),
+ "expected error message to name the rejected value, got: " + result);
Review Comment:
This passes even if the command is not registered at all.
`ShellEvaluationResultUtil.isSuccess` is just `!(x instanceof Throwable)`,
and Spring Shell 2.1.1 *returns* a `CommandNotFound` (a `RuntimeException`)
whose message echoes the input line verbatim. So a typo'd `@ShellMethod` key
yields `No command found for 'table set-meta-fields-mode --target-mode
BOGUS_MODE'`, which satisfies both assertions.
Assert on text only the production code can emit:
```suggestion
assertInstanceOf(HoodieException.class, result);
assertTrue(result.toString().contains("Unsupported --target-mode
'BOGUS_MODE'"),
"expected error message to name the rejected value, got: " + result);
```
(needs `org.junit.jupiter.api.Assertions.assertInstanceOf` and
`org.apache.hudi.exception.HoodieException` imports)
##########
hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestTableCommand.java:
##########
@@ -304,4 +305,140 @@ private String getFileContent(String fileToReadStr)
throws IOException {
fis.close();
return fromUTF8Bytes(data);
}
+
+ //
---------------------------------------------------------------------------
+ // set-meta-fields-mode
+ //
---------------------------------------------------------------------------
+
+ @Test
+ public void testSetMetaFieldsModeOnFreshTableToCommitTimeOnly() {
+ assertTrue(prepareTable());
+ // Default table is ALL — no commits yet, so the safety check must let
this through.
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_ONLY");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ // Rendered diff must surface the changed properties so the operator can
confirm the write.
+ String rendered = result.toString();
+ assertTrue(rendered.contains(HoodieTableConfig.POPULATE_META_FIELDS.key()),
+ "expected rendered diff to mention populate.meta.fields, got: " +
rendered);
+ assertTrue(rendered.contains(HoodieTableConfig.META_FIELDS_MODE.key()),
+ "expected rendered diff to mention meta.fields.mode, got: " +
rendered);
+ HoodieTableMetaClient client = HoodieCLI.getTableMetaClient();
+ assertEquals(MetaFieldsMode.COMMIT_TIME_ONLY,
client.getTableConfig().getMetaFieldsMode());
+ assertFalse(client.getTableConfig().populateMetaFields());
+ }
+
+ @Test
+ public void testSetMetaFieldsModeOnFreshTableToFileNameOnly() {
+ assertTrue(prepareTable());
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode FILE_NAME_ONLY");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ assertEquals(MetaFieldsMode.FILE_NAME_ONLY,
+ HoodieCLI.getTableMetaClient().getTableConfig().getMetaFieldsMode());
+ }
+
+ @Test
+ public void testSetMetaFieldsModeOnFreshTableToCombinedMode() {
+ assertTrue(prepareTable());
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_AND_FILE_NAME");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ assertEquals(MetaFieldsMode.COMMIT_TIME_AND_FILE_NAME,
+ HoodieCLI.getTableMetaClient().getTableConfig().getMetaFieldsMode());
+ }
+
+ @Test
+ public void testSetMetaFieldsModeOnFreshTableToNone() {
+ assertTrue(prepareTable());
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode NONE");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ HoodieTableMetaClient client = HoodieCLI.getTableMetaClient();
+ assertEquals(MetaFieldsMode.NONE,
client.getTableConfig().getMetaFieldsMode());
+ assertFalse(client.getTableConfig().populateMetaFields());
+ }
+
+ @Test
+ public void testSetMetaFieldsModeToAllClearsProperty() throws IOException {
+ assertTrue(prepareTable());
+ // First move to a selective mode so we have something to clear.
+ shell.evaluate(() -> "table set-meta-fields-mode --target-mode
COMMIT_TIME_ONLY");
+ // Then set back to ALL.
+ Object result = shell.evaluate(() -> "table set-meta-fields-mode
--target-mode ALL");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ HoodieTableMetaClient client = HoodieCLI.getTableMetaClient();
+ assertEquals(MetaFieldsMode.ALL,
client.getTableConfig().getMetaFieldsMode());
+ assertTrue(client.getTableConfig().populateMetaFields());
+ // The mode property should be cleared (ALL is implicit).
+
assertFalse(client.getTableConfig().getProps().containsKey(HoodieTableConfig.META_FIELDS_MODE.key()));
+ }
+
+ @Test
+ public void testSetMetaFieldsModeNoOpWhenAlreadyInTargetMode() {
+ assertTrue(prepareTable());
+ Object first = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_ONLY");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(first));
+ // Second call — same target — should be a no-op message.
+ Object second = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_ONLY");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(second));
+ assertTrue(second.toString().contains("already in COMMIT_TIME_ONLY"),
+ "expected no-op message, got: " + second);
+ }
+
+ @Test
+ public void testSetMetaFieldsModeRejectsUnknownValue() {
+ assertTrue(prepareTable());
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode BOGUS_MODE");
+ // Shell evaluate returns the exception object on failure.
+ assertFalse(ShellEvaluationResultUtil.isSuccess(result));
+ assertTrue(result.toString().contains("BOGUS_MODE"),
+ "expected error message to name the rejected value, got: " + result);
+ }
+
+ @Test
+ public void testSetMetaFieldsModeRefusesOnPopulatedTable() throws Exception {
+ assertTrue(prepareTable());
+ createDummyCommitFile("20260101000000000");
+ HoodieCLI.refreshTableMetadata();
+
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_ONLY");
+ assertFalse(ShellEvaluationResultUtil.isSuccess(result));
+ assertTrue(result.toString().contains("Refusing to change") ||
result.toString().contains("--force"),
+ "expected refusal message, got: " + result);
+
+ // Mode must not have changed.
+ assertEquals(MetaFieldsMode.ALL,
+ HoodieCLI.getTableMetaClient().getTableConfig().getMetaFieldsMode());
Review Comment:
Two things here.
The `||` is redundant slack -- I checked the plausible alternate failures
and none satisfy it today, but there is no upside to keeping it.
More importantly, line 415 reads `getTableConfig()` off the metaclient
snapshot taken *before* the command ran (`HoodieTableMetaClient` holds
`tableConfig` as a plain field), and the command throws at `:298` before its
final refresh. So this would read `ALL` even if the command had written to disk
and then thrown -- it cannot catch a write-before-guard regression.
`HoodieTableMetaClient.reload(...)` is the idiom used in
`TestRepairsCommand.java:218`.
```suggestion
assertTrue(result.toString().contains(
"Refusing to change hoodie.meta.fields.mode on a table that
already has 1 commit(s)"),
"expected refusal message, got: " + result);
// Mode must not have changed.
assertEquals(MetaFieldsMode.ALL,
HoodieTableMetaClient.reload(HoodieCLI.getTableMetaClient()).getTableConfig().getMetaFieldsMode());
```
##########
hudi-cli/src/main/java/org/apache/hudi/cli/commands/TableCommand.java:
##########
@@ -256,6 +258,82 @@ public String deleteTableConfig(
return renderOldNewProps(newProps, oldProps);
}
+ @ShellMethod(key = "table set-meta-fields-mode",
+ value = "Set hoodie.meta.fields.mode on an existing table. Refuses to
change the mode on a "
+ + "table that already has commits unless --force is passed — the
property is a "
+ + "physical-storage decision baked into files at write time, so
mixing modes across "
+ + "commits produces mixed-mode files whose incremental /
file-pruning semantics differ "
+ + "between old and new data. Use only on a table with zero commits,
or with --force if "
+ + "you accept the data-correctness consequences.")
+ public String setMetaFieldsMode(
+ @ShellOption(value = {"--target-mode"},
+ help = "One of ALL, NONE, COMMIT_TIME_ONLY, FILE_NAME_ONLY,
COMMIT_TIME_AND_FILE_NAME")
+ final String targetModeStr,
+ @ShellOption(value = {"--force"}, defaultValue = "false",
+ help = "Override the safety check that prevents changing the mode on
a table with commits. "
+ + "Existing files are not rewritten — new commits use the new
mode, old commits keep "
+ + "the old mode. Incremental queries and file-name-based lookups
will silently drop "
+ + "rows from commits written under the incompatible mode.")
+ final boolean force) throws IOException {
Review Comment:
Two nits, feel free to ignore:
- `throws IOException` is dead -- `HoodieTableConfig.update/delete` wrap in
unchecked `HoodieIOException`, and `refreshTableMetadata()` declares nothing.
- Lines 263 and 274 introduce the only non-ASCII characters in
`hudi-cli/src/main` (em dashes, in CLI help text). Suggest plain `--`.
##########
hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestTableCommand.java:
##########
@@ -304,4 +305,140 @@ private String getFileContent(String fileToReadStr)
throws IOException {
fis.close();
return fromUTF8Bytes(data);
}
+
+ //
---------------------------------------------------------------------------
+ // set-meta-fields-mode
+ //
---------------------------------------------------------------------------
+
+ @Test
+ public void testSetMetaFieldsModeOnFreshTableToCommitTimeOnly() {
+ assertTrue(prepareTable());
+ // Default table is ALL — no commits yet, so the safety check must let
this through.
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_ONLY");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ // Rendered diff must surface the changed properties so the operator can
confirm the write.
+ String rendered = result.toString();
+ assertTrue(rendered.contains(HoodieTableConfig.POPULATE_META_FIELDS.key()),
+ "expected rendered diff to mention populate.meta.fields, got: " +
rendered);
+ assertTrue(rendered.contains(HoodieTableConfig.META_FIELDS_MODE.key()),
+ "expected rendered diff to mention meta.fields.mode, got: " +
rendered);
+ HoodieTableMetaClient client = HoodieCLI.getTableMetaClient();
+ assertEquals(MetaFieldsMode.COMMIT_TIME_ONLY,
client.getTableConfig().getMetaFieldsMode());
+ assertFalse(client.getTableConfig().populateMetaFields());
+ }
+
+ @Test
+ public void testSetMetaFieldsModeOnFreshTableToFileNameOnly() {
+ assertTrue(prepareTable());
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode FILE_NAME_ONLY");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ assertEquals(MetaFieldsMode.FILE_NAME_ONLY,
+ HoodieCLI.getTableMetaClient().getTableConfig().getMetaFieldsMode());
+ }
+
+ @Test
+ public void testSetMetaFieldsModeOnFreshTableToCombinedMode() {
+ assertTrue(prepareTable());
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_AND_FILE_NAME");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ assertEquals(MetaFieldsMode.COMMIT_TIME_AND_FILE_NAME,
+ HoodieCLI.getTableMetaClient().getTableConfig().getMetaFieldsMode());
+ }
+
+ @Test
+ public void testSetMetaFieldsModeOnFreshTableToNone() {
+ assertTrue(prepareTable());
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode NONE");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ HoodieTableMetaClient client = HoodieCLI.getTableMetaClient();
+ assertEquals(MetaFieldsMode.NONE,
client.getTableConfig().getMetaFieldsMode());
+ assertFalse(client.getTableConfig().populateMetaFields());
+ }
+
+ @Test
+ public void testSetMetaFieldsModeToAllClearsProperty() throws IOException {
+ assertTrue(prepareTable());
+ // First move to a selective mode so we have something to clear.
+ shell.evaluate(() -> "table set-meta-fields-mode --target-mode
COMMIT_TIME_ONLY");
+ // Then set back to ALL.
+ Object result = shell.evaluate(() -> "table set-meta-fields-mode
--target-mode ALL");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ HoodieTableMetaClient client = HoodieCLI.getTableMetaClient();
+ assertEquals(MetaFieldsMode.ALL,
client.getTableConfig().getMetaFieldsMode());
+ assertTrue(client.getTableConfig().populateMetaFields());
+ // The mode property should be cleared (ALL is implicit).
+
assertFalse(client.getTableConfig().getProps().containsKey(HoodieTableConfig.META_FIELDS_MODE.key()));
+ }
+
+ @Test
+ public void testSetMetaFieldsModeNoOpWhenAlreadyInTargetMode() {
+ assertTrue(prepareTable());
+ Object first = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_ONLY");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(first));
+ // Second call — same target — should be a no-op message.
+ Object second = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_ONLY");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(second));
+ assertTrue(second.toString().contains("already in COMMIT_TIME_ONLY"),
+ "expected no-op message, got: " + second);
+ }
+
+ @Test
+ public void testSetMetaFieldsModeRejectsUnknownValue() {
+ assertTrue(prepareTable());
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode BOGUS_MODE");
+ // Shell evaluate returns the exception object on failure.
+ assertFalse(ShellEvaluationResultUtil.isSuccess(result));
+ assertTrue(result.toString().contains("BOGUS_MODE"),
+ "expected error message to name the rejected value, got: " + result);
+ }
+
+ @Test
+ public void testSetMetaFieldsModeRefusesOnPopulatedTable() throws Exception {
+ assertTrue(prepareTable());
+ createDummyCommitFile("20260101000000000");
+ HoodieCLI.refreshTableMetadata();
+
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_ONLY");
+ assertFalse(ShellEvaluationResultUtil.isSuccess(result));
+ assertTrue(result.toString().contains("Refusing to change") ||
result.toString().contains("--force"),
+ "expected refusal message, got: " + result);
+
+ // Mode must not have changed.
+ assertEquals(MetaFieldsMode.ALL,
+ HoodieCLI.getTableMetaClient().getTableConfig().getMetaFieldsMode());
+ }
+
+ @Test
+ public void testSetMetaFieldsModeWithForceOnPopulatedTable() throws
Exception {
+ assertTrue(prepareTable());
+ createDummyCommitFile("20260101000000000");
+ HoodieCLI.refreshTableMetadata();
+
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_ONLY --force
true");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ assertEquals(MetaFieldsMode.COMMIT_TIME_ONLY,
+ HoodieCLI.getTableMetaClient().getTableConfig().getMetaFieldsMode());
Review Comment:
These two assertions are a strict subset of
`testSetMetaFieldsModeOnFreshTableToCommitTimeOnly`. The only thing making this
a distinct scenario is that `createDummyCommitFile` produced a counted instant,
and nothing here pins that -- if the fixture ever stops being recognised, this
quietly becomes a duplicate of the fresh-table test and still passes green.
```suggestion
assertTrue(ShellEvaluationResultUtil.isSuccess(result));
// Pin the precondition: --force must be the branch actually under test.
assertEquals(1, HoodieCLI.getTableMetaClient().getActiveTimeline()
.getCommitsTimeline().countInstants());
assertEquals(MetaFieldsMode.COMMIT_TIME_ONLY,
HoodieCLI.getTableMetaClient().getTableConfig().getMetaFieldsMode());
```
##########
hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestTableCommand.java:
##########
@@ -304,4 +305,140 @@ private String getFileContent(String fileToReadStr)
throws IOException {
fis.close();
return fromUTF8Bytes(data);
}
+
+ //
---------------------------------------------------------------------------
+ // set-meta-fields-mode
+ //
---------------------------------------------------------------------------
+
+ @Test
+ public void testSetMetaFieldsModeOnFreshTableToCommitTimeOnly() {
+ assertTrue(prepareTable());
+ // Default table is ALL — no commits yet, so the safety check must let
this through.
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_ONLY");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ // Rendered diff must surface the changed properties so the operator can
confirm the write.
+ String rendered = result.toString();
+ assertTrue(rendered.contains(HoodieTableConfig.POPULATE_META_FIELDS.key()),
+ "expected rendered diff to mention populate.meta.fields, got: " +
rendered);
+ assertTrue(rendered.contains(HoodieTableConfig.META_FIELDS_MODE.key()),
+ "expected rendered diff to mention meta.fields.mode, got: " +
rendered);
+ HoodieTableMetaClient client = HoodieCLI.getTableMetaClient();
+ assertEquals(MetaFieldsMode.COMMIT_TIME_ONLY,
client.getTableConfig().getMetaFieldsMode());
+ assertFalse(client.getTableConfig().populateMetaFields());
+ }
+
+ @Test
+ public void testSetMetaFieldsModeOnFreshTableToFileNameOnly() {
+ assertTrue(prepareTable());
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode FILE_NAME_ONLY");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ assertEquals(MetaFieldsMode.FILE_NAME_ONLY,
+ HoodieCLI.getTableMetaClient().getTableConfig().getMetaFieldsMode());
+ }
+
+ @Test
+ public void testSetMetaFieldsModeOnFreshTableToCombinedMode() {
+ assertTrue(prepareTable());
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_AND_FILE_NAME");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ assertEquals(MetaFieldsMode.COMMIT_TIME_AND_FILE_NAME,
+ HoodieCLI.getTableMetaClient().getTableConfig().getMetaFieldsMode());
+ }
+
+ @Test
+ public void testSetMetaFieldsModeOnFreshTableToNone() {
+ assertTrue(prepareTable());
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode NONE");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ HoodieTableMetaClient client = HoodieCLI.getTableMetaClient();
+ assertEquals(MetaFieldsMode.NONE,
client.getTableConfig().getMetaFieldsMode());
+ assertFalse(client.getTableConfig().populateMetaFields());
+ }
+
+ @Test
+ public void testSetMetaFieldsModeToAllClearsProperty() throws IOException {
+ assertTrue(prepareTable());
+ // First move to a selective mode so we have something to clear.
+ shell.evaluate(() -> "table set-meta-fields-mode --target-mode
COMMIT_TIME_ONLY");
+ // Then set back to ALL.
+ Object result = shell.evaluate(() -> "table set-meta-fields-mode
--target-mode ALL");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ HoodieTableMetaClient client = HoodieCLI.getTableMetaClient();
+ assertEquals(MetaFieldsMode.ALL,
client.getTableConfig().getMetaFieldsMode());
+ assertTrue(client.getTableConfig().populateMetaFields());
+ // The mode property should be cleared (ALL is implicit).
+
assertFalse(client.getTableConfig().getProps().containsKey(HoodieTableConfig.META_FIELDS_MODE.key()));
+ }
+
+ @Test
+ public void testSetMetaFieldsModeNoOpWhenAlreadyInTargetMode() {
+ assertTrue(prepareTable());
+ Object first = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_ONLY");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(first));
+ // Second call — same target — should be a no-op message.
+ Object second = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_ONLY");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(second));
+ assertTrue(second.toString().contains("already in COMMIT_TIME_ONLY"),
+ "expected no-op message, got: " + second);
+ }
+
+ @Test
+ public void testSetMetaFieldsModeRejectsUnknownValue() {
+ assertTrue(prepareTable());
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode BOGUS_MODE");
+ // Shell evaluate returns the exception object on failure.
+ assertFalse(ShellEvaluationResultUtil.isSuccess(result));
+ assertTrue(result.toString().contains("BOGUS_MODE"),
+ "expected error message to name the rejected value, got: " + result);
+ }
+
+ @Test
+ public void testSetMetaFieldsModeRefusesOnPopulatedTable() throws Exception {
+ assertTrue(prepareTable());
+ createDummyCommitFile("20260101000000000");
+ HoodieCLI.refreshTableMetadata();
+
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_ONLY");
+ assertFalse(ShellEvaluationResultUtil.isSuccess(result));
+ assertTrue(result.toString().contains("Refusing to change") ||
result.toString().contains("--force"),
+ "expected refusal message, got: " + result);
+
+ // Mode must not have changed.
+ assertEquals(MetaFieldsMode.ALL,
+ HoodieCLI.getTableMetaClient().getTableConfig().getMetaFieldsMode());
+ }
+
+ @Test
+ public void testSetMetaFieldsModeWithForceOnPopulatedTable() throws
Exception {
+ assertTrue(prepareTable());
+ createDummyCommitFile("20260101000000000");
+ HoodieCLI.refreshTableMetadata();
+
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_ONLY --force
true");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ assertEquals(MetaFieldsMode.COMMIT_TIME_ONLY,
+ HoodieCLI.getTableMetaClient().getTableConfig().getMetaFieldsMode());
+ }
+
+ private void createDummyCommitFile(String instantTime) throws IOException {
+ // Timeline v2 layout: files live under .hoodie/timeline/. Writing an
empty completed commit
Review Comment:
This hand-rolls the v2 timeline layout when the helper already exists and is
already used in this same file at line 217:
`HoodieTestDataGenerator.createCommitFile(tablePath, instantTime,
storageConf())`. It is imported at line 37, and the sibling command's test does
the same thing (`ITTestTableCommand.java:81-82`).
The helper also writes real commit metadata and a valid instant time --
`instantTime + "1"` here is an 18-char string that is not a valid Hudi
timestamp.
Suggest deleting this method and calling the helper at both call sites (405
and 422). That also removes the fully-qualified `java.nio.file.*` references.
##########
hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestTableCommand.java:
##########
@@ -304,4 +305,140 @@ private String getFileContent(String fileToReadStr)
throws IOException {
fis.close();
return fromUTF8Bytes(data);
}
+
+ //
---------------------------------------------------------------------------
+ // set-meta-fields-mode
+ //
---------------------------------------------------------------------------
+
+ @Test
+ public void testSetMetaFieldsModeOnFreshTableToCommitTimeOnly() {
+ assertTrue(prepareTable());
+ // Default table is ALL — no commits yet, so the safety check must let
this through.
+ Object result = shell.evaluate(() ->
+ "table set-meta-fields-mode --target-mode COMMIT_TIME_ONLY");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+ // Rendered diff must surface the changed properties so the operator can
confirm the write.
+ String rendered = result.toString();
+ assertTrue(rendered.contains(HoodieTableConfig.POPULATE_META_FIELDS.key()),
+ "expected rendered diff to mention populate.meta.fields, got: " +
rendered);
+ assertTrue(rendered.contains(HoodieTableConfig.META_FIELDS_MODE.key()),
+ "expected rendered diff to mention meta.fields.mode, got: " +
rendered);
+ HoodieTableMetaClient client = HoodieCLI.getTableMetaClient();
+ assertEquals(MetaFieldsMode.COMMIT_TIME_ONLY,
client.getTableConfig().getMetaFieldsMode());
+ assertFalse(client.getTableConfig().populateMetaFields());
+ }
+
+ @Test
Review Comment:
nit, feel free to ignore: these three fresh-table tests differ only by the
mode string. `@ParameterizedTest` + `@EnumSource(names = {"COMMIT_TIME_ONLY",
"FILE_NAME_ONLY", "COMMIT_TIME_AND_FILE_NAME"})` would collapse them and let
all three carry the full assertion set -- right now `FILE_NAME_ONLY` and
`COMMIT_TIME_AND_FILE_NAME` skip the `assertFalse(populateMetaFields())` check
the first one has. That check stops being implied once #19205's `resolve()`
lands, since it no longer consults the boolean.
Keep `...ToNone` separate (it deletes the key rather than writing it), and
do not hoist `prepareTable()` into `@BeforeEach` --
`testCreateWithSpecifiedValues` and `testRefresh` would start failing with
"Table already existing".
##########
hudi-cli/src/main/java/org/apache/hudi/cli/commands/TableCommand.java:
##########
@@ -256,6 +258,82 @@ public String deleteTableConfig(
return renderOldNewProps(newProps, oldProps);
}
+ @ShellMethod(key = "table set-meta-fields-mode",
+ value = "Set hoodie.meta.fields.mode on an existing table. Refuses to
change the mode on a "
+ + "table that already has commits unless --force is passed — the
property is a "
+ + "physical-storage decision baked into files at write time, so
mixing modes across "
+ + "commits produces mixed-mode files whose incremental /
file-pruning semantics differ "
+ + "between old and new data. Use only on a table with zero commits,
or with --force if "
+ + "you accept the data-correctness consequences.")
+ public String setMetaFieldsMode(
+ @ShellOption(value = {"--target-mode"},
+ help = "One of ALL, NONE, COMMIT_TIME_ONLY, FILE_NAME_ONLY,
COMMIT_TIME_AND_FILE_NAME")
+ final String targetModeStr,
+ @ShellOption(value = {"--force"}, defaultValue = "false",
+ help = "Override the safety check that prevents changing the mode on
a table with commits. "
+ + "Existing files are not rewritten — new commits use the new
mode, old commits keep "
+ + "the old mode. Incremental queries and file-name-based lookups
will silently drop "
+ + "rows from commits written under the incompatible mode.")
+ final boolean force) throws IOException {
+ MetaFieldsMode targetMode;
+ try {
+ targetMode = MetaFieldsMode.valueOf(targetModeStr.trim());
+ } catch (IllegalArgumentException e) {
+ throw new HoodieException(String.format(
+ "Unsupported --target-mode '%s'. Allowed values: ALL, NONE,
COMMIT_TIME_ONLY, "
+ + "FILE_NAME_ONLY, COMMIT_TIME_AND_FILE_NAME.", targetModeStr));
+ }
+
+ HoodieCLI.refreshTableMetadata();
+ HoodieTableMetaClient client = HoodieCLI.getTableMetaClient();
+ Map<String, String> oldProps = client.getTableConfig().propsMap();
+ MetaFieldsMode currentMode = client.getTableConfig().getMetaFieldsMode();
+
+ if (currentMode == targetMode) {
+ return String.format("Table is already in %s mode; nothing to change.",
targetMode);
+ }
+
+ // Safety check: refuse to change the mode on a table with commits unless
--force.
+ int commitCount =
client.getActiveTimeline().getCommitsTimeline().countInstants();
Review Comment:
This lets you set a selective mode on a MoR table, and on a fresh table
there are no commits so `--force` is not even needed. The writer already
rejects that combination.
`HoodieWriteConfig.java:3936-3947` (added by this stack's own base commit
`2b11a4fa`) throws for `MERGE_ON_READ + selective` and for `engine != SPARK +
selective`. `HoodieSparkSqlWriter.scala:1102-1106` copies every table property
into the write config, so once this command has written the mode, **every**
subsequent write fails validation. The table is bricked and the CLI gave no
warning.
Please reject selective modes up front when the table is MoR (or the writer
is not Spark), reusing the writer's message. `--force` should not override this
one -- it is an unimplemented code path, not a data-correctness tradeoff.
Worth a `create --tableType MERGE_ON_READ` case too; all 9 new tests go
through `prepareTable()`, which is CoW only.
##########
hudi-cli/src/main/java/org/apache/hudi/cli/commands/TableCommand.java:
##########
@@ -256,6 +258,82 @@ public String deleteTableConfig(
return renderOldNewProps(newProps, oldProps);
}
+ @ShellMethod(key = "table set-meta-fields-mode",
+ value = "Set hoodie.meta.fields.mode on an existing table. Refuses to
change the mode on a "
+ + "table that already has commits unless --force is passed — the
property is a "
+ + "physical-storage decision baked into files at write time, so
mixing modes across "
+ + "commits produces mixed-mode files whose incremental /
file-pruning semantics differ "
+ + "between old and new data. Use only on a table with zero commits,
or with --force if "
+ + "you accept the data-correctness consequences.")
+ public String setMetaFieldsMode(
+ @ShellOption(value = {"--target-mode"},
+ help = "One of ALL, NONE, COMMIT_TIME_ONLY, FILE_NAME_ONLY,
COMMIT_TIME_AND_FILE_NAME")
+ final String targetModeStr,
+ @ShellOption(value = {"--force"}, defaultValue = "false",
+ help = "Override the safety check that prevents changing the mode on
a table with commits. "
+ + "Existing files are not rewritten — new commits use the new
mode, old commits keep "
+ + "the old mode. Incremental queries and file-name-based lookups
will silently drop "
+ + "rows from commits written under the incompatible mode.")
+ final boolean force) throws IOException {
+ MetaFieldsMode targetMode;
+ try {
+ targetMode = MetaFieldsMode.valueOf(targetModeStr.trim());
+ } catch (IllegalArgumentException e) {
+ throw new HoodieException(String.format(
+ "Unsupported --target-mode '%s'. Allowed values: ALL, NONE,
COMMIT_TIME_ONLY, "
+ + "FILE_NAME_ONLY, COMMIT_TIME_AND_FILE_NAME.", targetModeStr));
+ }
+
+ HoodieCLI.refreshTableMetadata();
+ HoodieTableMetaClient client = HoodieCLI.getTableMetaClient();
+ Map<String, String> oldProps = client.getTableConfig().propsMap();
+ MetaFieldsMode currentMode = client.getTableConfig().getMetaFieldsMode();
+
+ if (currentMode == targetMode) {
+ return String.format("Table is already in %s mode; nothing to change.",
targetMode);
+ }
+
+ // Safety check: refuse to change the mode on a table with commits unless
--force.
+ int commitCount =
client.getActiveTimeline().getCommitsTimeline().countInstants();
+ if (commitCount > 0 && !force) {
+ throw new HoodieException(String.format(
+ "Refusing to change hoodie.meta.fields.mode on a table that already
has %d commit(s). "
+ + "Existing files were written under %s and will not be
rewritten by this command; "
+ + "new commits would be written under %s, producing mixed-mode
files whose "
+ + "incremental / file-pruning semantics differ between old and
new data. "
+ + "Pass --force if you accept the consequences, or recreate the
table to change "
+ + "the mode cleanly.",
+ commitCount, currentMode, targetMode));
+ }
+ if (commitCount > 0) {
+ log.warn("--force passed: changing hoodie.meta.fields.mode from {} to {}
on a table with "
Review Comment:
The warning undersells this. Widening (`COMMIT_TIME_ONLY -> ALL`, `NONE ->
anything`) does not silently drop rows -- it throws.
Under a selective mode `_hoodie_record_key` is a physically present NULL
column. Once this sets `populate.meta.fields=true`, `RecordContext.java:76-78`
switches to `metadataKeyExtractor()`, which is an unconditional `.toString()`
on that null (`RecordContext.java:438-439`). Every read of a pre-existing file
group NPEs.
It is also the one way to defeat the guard permanently:
`BaseHoodieWriteClient.java:1552` compares *writer vs table* config, so
rewriting the table config means it can never fire again. That invariant goes
back to `d5026e9a2485` (HUDI-2161, 2021), and #19205 restates it per-mode via
`isWiderThan`.
Suggest making widening a hard refusal that `--force` cannot override, and
keeping `--force` for narrowing only.
##########
hudi-cli/src/main/java/org/apache/hudi/cli/commands/TableCommand.java:
##########
@@ -256,6 +258,82 @@ public String deleteTableConfig(
return renderOldNewProps(newProps, oldProps);
}
+ @ShellMethod(key = "table set-meta-fields-mode",
+ value = "Set hoodie.meta.fields.mode on an existing table. Refuses to
change the mode on a "
+ + "table that already has commits unless --force is passed — the
property is a "
+ + "physical-storage decision baked into files at write time, so
mixing modes across "
+ + "commits produces mixed-mode files whose incremental /
file-pruning semantics differ "
+ + "between old and new data. Use only on a table with zero commits,
or with --force if "
+ + "you accept the data-correctness consequences.")
+ public String setMetaFieldsMode(
+ @ShellOption(value = {"--target-mode"},
+ help = "One of ALL, NONE, COMMIT_TIME_ONLY, FILE_NAME_ONLY,
COMMIT_TIME_AND_FILE_NAME")
+ final String targetModeStr,
+ @ShellOption(value = {"--force"}, defaultValue = "false",
+ help = "Override the safety check that prevents changing the mode on
a table with commits. "
+ + "Existing files are not rewritten — new commits use the new
mode, old commits keep "
+ + "the old mode. Incremental queries and file-name-based lookups
will silently drop "
+ + "rows from commits written under the incompatible mode.")
+ final boolean force) throws IOException {
+ MetaFieldsMode targetMode;
+ try {
+ targetMode = MetaFieldsMode.valueOf(targetModeStr.trim());
+ } catch (IllegalArgumentException e) {
+ throw new HoodieException(String.format(
+ "Unsupported --target-mode '%s'. Allowed values: ALL, NONE,
COMMIT_TIME_ONLY, "
+ + "FILE_NAME_ONLY, COMMIT_TIME_AND_FILE_NAME.", targetModeStr));
+ }
+
+ HoodieCLI.refreshTableMetadata();
+ HoodieTableMetaClient client = HoodieCLI.getTableMetaClient();
+ Map<String, String> oldProps = client.getTableConfig().propsMap();
+ MetaFieldsMode currentMode = client.getTableConfig().getMetaFieldsMode();
+
+ if (currentMode == targetMode) {
+ return String.format("Table is already in %s mode; nothing to change.",
targetMode);
+ }
+
+ // Safety check: refuse to change the mode on a table with commits unless
--force.
+ int commitCount =
client.getActiveTimeline().getCommitsTimeline().countInstants();
+ if (commitCount > 0 && !force) {
+ throw new HoodieException(String.format(
+ "Refusing to change hoodie.meta.fields.mode on a table that already
has %d commit(s). "
+ + "Existing files were written under %s and will not be
rewritten by this command; "
+ + "new commits would be written under %s, producing mixed-mode
files whose "
+ + "incremental / file-pruning semantics differ between old and
new data. "
+ + "Pass --force if you accept the consequences, or recreate the
table to change "
+ + "the mode cleanly.",
+ commitCount, currentMode, targetMode));
+ }
+ if (commitCount > 0) {
+ log.warn("--force passed: changing hoodie.meta.fields.mode from {} to {}
on a table with "
+ + "{} commit(s). Existing files retain the old layout; new
commits use the new "
+ + "layout. Incremental queries and file-name lookups may
silently drop rows written "
+ + "under the incompatible mode.",
+ currentMode, targetMode, commitCount);
+ }
+
+ // Persist. ALL and NONE are implicit (derived from populate.meta.fields);
selective modes are
+ // written explicitly. We also toggle populate.meta.fields to keep both
properties in sync so
+ // older readers that only understand the legacy boolean still degrade
correctly.
+ Properties toUpdate = new Properties();
+ toUpdate.setProperty(HoodieTableConfig.POPULATE_META_FIELDS.key(),
+ String.valueOf(targetMode == MetaFieldsMode.ALL));
+ if (targetMode == MetaFieldsMode.ALL || targetMode == MetaFieldsMode.NONE)
{
+ // Selective mode is being cleared; delete the property rather than
write empty string.
+ HoodieTableConfig.delete(client.getStorage(), client.getMetaPath(),
+ Collections.singleton(HoodieTableConfig.META_FIELDS_MODE.key()));
+ HoodieTableConfig.update(client.getStorage(), client.getMetaPath(),
toUpdate);
+ } else {
+ toUpdate.setProperty(HoodieTableConfig.META_FIELDS_MODE.key(),
targetMode.name());
+ HoodieTableConfig.update(client.getStorage(), client.getMetaPath(),
toUpdate);
+ }
Review Comment:
These are two independent `modify()` cycles
(`HoodieTableConfig.java:552-600`), and the state between them is `NONE` --
meta fields off entirely, worse than either endpoint. If the second call fails
the table stays there: `modify()` drops its backup on success, and
`recoverIfNeeded` only kicks in when `hoodie.properties` is missing or invalid,
so `table recover-configs` cannot undo it.
That breaks the one-transaction-per-mutation invariant from `ae0c67d9fc7c`
(HUDI-2795). `updateAndDeleteProps` already exists for exactly this "set some,
delete some" shape (`HoodieTableConfig.java:616`, added by `b60d38c40fa1`).
```suggestion
Properties toUpdate = new Properties();
toUpdate.setProperty(HoodieTableConfig.POPULATE_META_FIELDS.key(),
String.valueOf(targetMode == MetaFieldsMode.ALL));
boolean implicitMode = targetMode == MetaFieldsMode.ALL || targetMode ==
MetaFieldsMode.NONE;
if (!implicitMode) {
toUpdate.setProperty(HoodieTableConfig.META_FIELDS_MODE.key(),
targetMode.name());
}
// Single modify() cycle: never leave the table in an intermediate mode.
HoodieTableConfig.updateAndDeleteProps(client.getStorage(),
client.getMetaPath(), toUpdate,
implicitMode ?
Collections.singleton(HoodieTableConfig.META_FIELDS_MODE.key())
: Collections.emptySet());
```
If you take the rebase fix in my other comment, the delete goes away
entirely and this collapses to a plain `update()`.
##########
hudi-cli/src/main/java/org/apache/hudi/cli/commands/TableCommand.java:
##########
@@ -256,6 +258,82 @@ public String deleteTableConfig(
return renderOldNewProps(newProps, oldProps);
}
+ @ShellMethod(key = "table set-meta-fields-mode",
+ value = "Set hoodie.meta.fields.mode on an existing table. Refuses to
change the mode on a "
+ + "table that already has commits unless --force is passed — the
property is a "
+ + "physical-storage decision baked into files at write time, so
mixing modes across "
+ + "commits produces mixed-mode files whose incremental /
file-pruning semantics differ "
+ + "between old and new data. Use only on a table with zero commits,
or with --force if "
+ + "you accept the data-correctness consequences.")
+ public String setMetaFieldsMode(
+ @ShellOption(value = {"--target-mode"},
+ help = "One of ALL, NONE, COMMIT_TIME_ONLY, FILE_NAME_ONLY,
COMMIT_TIME_AND_FILE_NAME")
+ final String targetModeStr,
+ @ShellOption(value = {"--force"}, defaultValue = "false",
+ help = "Override the safety check that prevents changing the mode on
a table with commits. "
+ + "Existing files are not rewritten — new commits use the new
mode, old commits keep "
+ + "the old mode. Incremental queries and file-name-based lookups
will silently drop "
+ + "rows from commits written under the incompatible mode.")
+ final boolean force) throws IOException {
+ MetaFieldsMode targetMode;
+ try {
+ targetMode = MetaFieldsMode.valueOf(targetModeStr.trim());
Review Comment:
`valueOf` is case-sensitive, so `--target-mode all` fails. #19205 added
`MetaFieldsMode.parse()` (case-insensitive, `7dcd9d1b`) at danny0405's request
precisely for user-supplied input -- and the CLI is the most user-facing input
surface we have. After the rebase this would be the only entry point still
rejecting lowercase.
There are also three different "allowed values" lists now: this one, the
enum's (which omits `ALL`), and the config doc at
`HoodieTableConfig.java:340-342`. Switching to
`MetaFieldsMode.parse(targetModeStr)` and deleting the local try/catch
collapses them to one.
##########
hudi-cli/src/main/java/org/apache/hudi/cli/commands/TableCommand.java:
##########
@@ -256,6 +258,82 @@ public String deleteTableConfig(
return renderOldNewProps(newProps, oldProps);
}
+ @ShellMethod(key = "table set-meta-fields-mode",
+ value = "Set hoodie.meta.fields.mode on an existing table. Refuses to
change the mode on a "
+ + "table that already has commits unless --force is passed — the
property is a "
+ + "physical-storage decision baked into files at write time, so
mixing modes across "
+ + "commits produces mixed-mode files whose incremental /
file-pruning semantics differ "
+ + "between old and new data. Use only on a table with zero commits,
or with --force if "
+ + "you accept the data-correctness consequences.")
+ public String setMetaFieldsMode(
+ @ShellOption(value = {"--target-mode"},
+ help = "One of ALL, NONE, COMMIT_TIME_ONLY, FILE_NAME_ONLY,
COMMIT_TIME_AND_FILE_NAME")
+ final String targetModeStr,
+ @ShellOption(value = {"--force"}, defaultValue = "false",
+ help = "Override the safety check that prevents changing the mode on
a table with commits. "
+ + "Existing files are not rewritten — new commits use the new
mode, old commits keep "
+ + "the old mode. Incremental queries and file-name-based lookups
will silently drop "
+ + "rows from commits written under the incompatible mode.")
+ final boolean force) throws IOException {
+ MetaFieldsMode targetMode;
+ try {
+ targetMode = MetaFieldsMode.valueOf(targetModeStr.trim());
+ } catch (IllegalArgumentException e) {
+ throw new HoodieException(String.format(
+ "Unsupported --target-mode '%s'. Allowed values: ALL, NONE,
COMMIT_TIME_ONLY, "
+ + "FILE_NAME_ONLY, COMMIT_TIME_AND_FILE_NAME.", targetModeStr));
+ }
+
+ HoodieCLI.refreshTableMetadata();
+ HoodieTableMetaClient client = HoodieCLI.getTableMetaClient();
+ Map<String, String> oldProps = client.getTableConfig().propsMap();
+ MetaFieldsMode currentMode = client.getTableConfig().getMetaFieldsMode();
+
+ if (currentMode == targetMode) {
+ return String.format("Table is already in %s mode; nothing to change.",
targetMode);
+ }
+
+ // Safety check: refuse to change the mode on a table with commits unless
--force.
+ int commitCount =
client.getActiveTimeline().getCommitsTimeline().countInstants();
+ if (commitCount > 0 && !force) {
+ throw new HoodieException(String.format(
+ "Refusing to change hoodie.meta.fields.mode on a table that already
has %d commit(s). "
+ + "Existing files were written under %s and will not be
rewritten by this command; "
+ + "new commits would be written under %s, producing mixed-mode
files whose "
+ + "incremental / file-pruning semantics differ between old and
new data. "
+ + "Pass --force if you accept the consequences, or recreate the
table to change "
+ + "the mode cleanly.",
+ commitCount, currentMode, targetMode));
+ }
+ if (commitCount > 0) {
+ log.warn("--force passed: changing hoodie.meta.fields.mode from {} to {}
on a table with "
+ + "{} commit(s). Existing files retain the old layout; new
commits use the new "
+ + "layout. Incremental queries and file-name lookups may
silently drop rows written "
+ + "under the incompatible mode.",
+ currentMode, targetMode, commitCount);
+ }
+
+ // Persist. ALL and NONE are implicit (derived from populate.meta.fields);
selective modes are
Review Comment:
This PR is stacked on a snapshot of #19205 that is now 9 commits behind, and
it encodes the semantics #19205 has since dropped.
`c78dc962` + `664ff2ec` made `hoodie.meta.fields.mode` the source of truth,
persisted verbatim including `ALL` and `NONE`, with the legacy boolean derived
from it -- that was the fix for danny0405's P1. And `4134dac6`'s
`NineToTenUpgradeHandler` now writes the property explicitly onto exactly the
tables this command would strip it from.
Please rebase and drop the clearing branch: always write
`META_FIELDS_MODE=targetMode.name()` plus the derived boolean, in one
`update()`. The assertion at `TestTableCommand.java:374` pins the old
representation and will need inverting.
--
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]
