nsivabalan commented on PR #19205: URL: https://github.com/apache/hudi/pull/19205#issuecomment-5221948330
@voonhous @cshuo @danny0405 — ready for another look. Summarising what moved, since this has turned over several times and I would rather not have you reconstruct it from the thread. ### Start here **The PR description is now the authoritative statement of behavior.** Four numbered sections: how a single config resolves (with the full truth table), why the mode is a table property, which transitions are legal, and upgrade/downgrade expectations. Earlier revisions of that text described models that no longer hold — if something in an old comment of mine contradicts it, the description wins. ### What changed since you last read this **@cshuo** — your restart concern is fixed and now tested end to end, but the fix moved. It is no longer in `StreamSync` (that back-fill is deleted); it is one rule in `BaseHoodieWriteClient` covering every engine: a writer stating *neither* meta-field property inherits the table's mode, one stating *either* is compared and rejected. Two streamer tests cover it — restart stating nothing (inherits, keeps writing commit times) and restart stating `populate.meta.fields=false` (rejected, table untouched). The second is your exact scenario, and it is a breaking change relative to today. **@voonhous** — four things: 1. **You were right that the missing incremental test was the highest-value gap.** Writing it surfaced two live bugs: the MoR guard was rejecting *every* selective CoW table, and the CoW commit-time guard sat on a path the datasource never takes, so `FILE_NAME_ONLY`/`NONE` returned zero rows silently. The feature did not work before this. Neither was findable by reading. 2. **Your clustering critique was exactly right** — `assertNotNull` passed on the buggy value. Rewritten to compare `_hoodie_file_name` against `input_file_name()` per row plus assert a replacecommit exists. 3. **Your one-way downgrade objection is resolved, but not the way I first said.** I made it throw, then reverted that — the throw guarded a state that cannot occur, since every selective mode already persists `populate.meta.fields=false`. Downgrade now proceeds with a warning and the round trip is pinned by a test. 4. **I audited all ten of your test asks against the tree and found I had missed one** — #37 asked for the restart to state `POPULATE_META_FIELDS=false`; mine stated nothing. Those were equivalent when you wrote it and are not any more. Fixed in `e71d4d4e3053`. One deviation remains on #36, with rationale on the thread. **@danny0405** — I have reopened your two P1 threads. Both are still fixed, but the mechanism changed materially after you approved my replies, and on the exact surface each P1 was about. Worth re-reading those two specifically. ### Three behavior changes for release notes 1. Row-writer meta-column stubs are SQL `NULL` instead of `""`, including on existing `populate.meta.fields=false` tables. 2. **`hoodie.populate.meta.fields=false` against an `ALL` table now throws** where it previously narrowed silently (retires HUDI-2161). This is the one change that can fail a working pipeline. 3. A selective mode does not survive a v10 → v9 → v10 round trip. Plus: a contradictory `mode` + `populate` pair is now rejected at table creation rather than silently overridden. ### Not in this PR, deliberately A pre-existing bug found while testing: `count()` and projected reads return **0 rows** on CoW incremental queries while `collect()` works — reproduced on unmodified `master` on a default `ALL` table. It is on the shared read path for every Spark query, so it needs its own JIRA and review rather than riding in here. Details and the repro are in the description under Follow-ups. CI has not run against the latest commits yet. -- 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]
