nsivabalan commented on PR #19206: URL: https://github.com/apache/hudi/pull/19206#issuecomment-5219365673
Rebased onto #19205's current head, and tightened the command to match the rule that PR establishes. Previously this branch was stacked on `13b39db9fb22`, 17 commits behind. It now sits on top of all of #19205, so the two read as a clean stack: **#19205 makes `hoodie.meta.fields.mode` a table property a write can never change; this PR provides the only sanctioned way to change it.** ### Three gaps closed against that rule **1. Widening was allowed.** The `--force` gate keyed only on commit count, so `--target-mode ALL --force` on a populated `COMMIT_TIME_ONLY` table went through. Existing files are not rewritten, so the table would advertise a meta column that is physically null for every earlier row — and incremental queries would then admit the table and silently skip exactly those rows. Widening is now **refused outright, and `--force` does not override it**. The asymmetry is deliberate: - *Narrowing* leaves earlier files carrying values nothing reads. A reader can ignore them, so `--force` remains the right gate — the operator is accepting mixed-mode files. - *Widening* leaves later files claiming a column earlier files do not have, with nothing to distinguish the two sets. There is no consequence an operator can knowingly accept, so it is a hard failure. This uses `MetaFieldsMode#isWiderThan` — the same predicate `BaseHoodieWriteClient#validateAgainstTableProperties` uses in #19205 — so the CLI and the write path cannot drift on which transitions are legal. **2. `ALL` and `NONE` deleted the mode property rather than writing it.** Three problems: the table then resolved through the legacy fallback, indistinguishable from a table predating the property; the command's effect became invisible to anyone reading `hoodie.properties`; and `TenToNineDowngradeHandler` reads the mode to derive the boolean it writes back on downgrade. Both properties are now always written, with the boolean **derived** from the mode — the same invariant `HoodieTableMetaClient.TableBuilder` enforces at creation, so `hoodie.properties` can never contradict itself. **3. `valueOf` rejected lowercase and produced a bare `No enum constant`.** Routed through `MetaFieldsMode.parse`, so `--target-mode commit_time_only` works and the error lists the allowed values like every other path that parses this property. ### Tests Five new cases: widening refused in three forms (including both `--force` variants), narrowing refused without `--force` and accepted with it, lowercase accepted, and both properties in agreement across all five modes. `testSetMetaFieldsModeToAllClearsProperty` asserted the deleted-property behavior from gap 2, so it is rewritten as `testSetMetaFieldsModeToAllWritesTheModeExplicitly` rather than deleted — the old name pinned a decision that turned out to be wrong, and the new one pins the corrected invariant. ### Note on local build `hudi-cli` compilation is currently blocked locally by a **stale `hudi-utilities-bundle`** that embeds an older shaded `MetaFieldsMode` and shadows `hudi-common` on the classpath; rebuilding that bundle fails on an unrelated pre-existing error in `KafkaAvroSchemaDeserializer` (reproduces on `master`). I verified `TableCommand.java` and `TestTableCommand.java` compile cleanly against a classpath with the stale bundle excluded, and checkstyle passes on `hudi-cli`. CI builds everything fresh so it should not hit this, but flagging it in case anyone sees the same locally. -- 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]
