nsivabalan commented on PR #19205: URL: https://github.com/apache/hudi/pull/19205#issuecomment-5219214790
Two scope callouts, so neither gets mistaken for something this PR should have handled. ### 1. hudi-cli is a follow-up (#19206) `HoodieTableConfig:352` and the error messages in `HoodieWriterUtils.scala` / `BaseHoodieWriteClient` direct users to hudi-cli for changing `hoodie.meta.fields.mode`, but the `table set-meta-fields-mode` command lives in **#19206**, not in this tree. That is deliberate — this PR keeps the config + writer + read path, and the CLI ships separately — but it means that until #19206 lands, the only in-tree ways to set the mode are table creation and upgrade. #19206 is up for review and I am rebasing it onto this branch's current head next; it is currently stacked on `13b39db9fb22`, which is 17 commits behind. The one-way rule this PR establishes applies there too: the CLI may **narrow** a mode and must reject a widening, since dropping a meta column leaves earlier files carrying values nothing reads (recoverable) while adding one leaves later files claiming a column earlier files lack (not recoverable). `MetaFieldsMode#isWiderThan` is the shared predicate and its javadoc now says so. ### 2. The CoW incremental `count()` bug is pre-existing and out of scope Restating this plainly because I initially misattributed it to this patch: **it is not caused by this PR and should be fixed outside it.** `count()` and column-projected reads return 0 rows on a CoW incremental query while `collect()` returns the correct rows. Reproduced on unmodified `master`, table version 9, on a **default `ALL` table** — no `hoodie.meta.fields.mode` anywhere in the tree: ``` MASTER_PROBE tableVersion=NINE collect=2 count=0 selectCol1=0 ``` Suspected cause: `HoodieFileGroupReaderBasedFileFormat` forces `isCount` false for incremental reads (`:253`), so a column-less scan falls through the guard at `:304` to `readBaseFile`, which hands the `_hoodie_commit_time` predicate to Parquet alongside an empty read schema. Parquet then evaluates a predicate on a column it is not reading and drops everything. I am not fixing it here. It sits on the shared read path for every Spark query — snapshot, incremental, CDC, bootstrap, MoR — so a wrong fix has far more blast radius than anything else in this PR, and this PR is already large. I will triage it separately once this lands. **No test is disabled for it.** The incremental tests here assert on `collectAsList()` rather than `count()`, which sidesteps it entirely, and there is now a comment in `TestMetaFieldsModeE2E` explaining the constraint so the next person adding a case does not trip over it. If reviewers would rather see a `@Disabled` test documenting the bug in-tree, I am happy to add one — but I did not want to land a knowingly-red test. -- 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]
