nsivabalan opened a new pull request, #19378:
URL: https://github.com/apache/hudi/pull/19378

   ### Describe the issue this Pull Request addresses
   
   Stacked on #19205 — please review and merge that one first.
   
   While scoping the `hoodie.populate.meta.fields` deprecation follow-up 
requested in review of #19205, I audited all 103 `populateMetaFields()` / 
`POPULATE_META_FIELDS` references in `src/main`. Most are semantically 
identical today and only misname intent (those are the rename sweep, still to 
come). But a dozen sites read the deprecated boolean *directly* rather than 
resolving `hoodie.meta.fields.mode` — and with a selective mode set while the 
boolean sits at its `true` default, they resolve the wrong way. Several are 
reachable on the Spark CoW path #19205 enables.
   
   This PR fixes those.
   
   ### Summary and Changelog
   
   **Record-key reads → NPE.** Under a selective mode these take the 
meta-column branch and then dereference the null `_hoodie_record_key` ordinal:
   - `HoodieSparkRecord#wrapIntoHoodieRecordPayloadWithKeyGen`
   - `HoodieAvroIndexedRecord#wrapIntoHoodieRecordPayloadWithKeyGen`
   
   **Writer factories bypassing the mode.** Would stamp all five meta columns 
and enable the bloom filter — the opposite of the requested mode:
   - `HoodieSparkFileWriterFactory` Lance + Vortex paths (the parquet path 
beside them was already migrated)
   - `HoodieAvroFileWriterFactory` HFile path — its `getBoolean` (rather than 
`getBooleanOrDefault`) could also NPE on unbox when neither property is set
   
   **Schema reconstruction asking the wrong question.** These ask "are the meta 
columns on disk?" using an is-record-key-populated predicate. Selective modes 
write the meta columns as physical *nullable* columns, so the correct test is 
`mode != NONE` — the predicate `TableSchemaResolver` already uses. Otherwise 
the rebuilt schema omits columns the file actually has:
   - `SparkValidatorUtils` — that schema feeds straight into 
`spark.read().schema(...).parquet(...)`
   - `PartitionStatsIndexer`, `SparkMetadataWriterUtils`
   
   **Guards bypassable by setting only the mode:**
   - `AutoRecordKeyGenerationUtils` — auto-generated keys computed, then 
discarded on a table that does not populate `_hoodie_record_key`
   - `DataSourceOptions` `ENABLE_ROW_WRITER` infer fn — row writer stayed on, 
silently ignoring `COMBINE_BEFORE_INSERT`, the exact case its own comment warns 
about
   - `BaseDatasetBulkInsertCommitActionExecutor` — picked a partitioner sorting 
on null `_hoodie_record_key` / `_hoodie_partition_path`
   
   **Bootstrap dropped the mode entirely.** `BootstrapExecutorUtils` and 
`BootstrapExecutor` called `setPopulateMetaFields(...)` without the paired 
`setMetaFieldsModeFromString(...)` that `HoodieSparkSqlWriter` and `StreamSync` 
have.
   
   **Irreversibility guard was all-or-nothing.** In 
`BaseHoodieWriteClient#validateAgainstTableProperties`, both sides of 
`!tableConfig.populateMetaFields() && writeConfig.populateMetaFields()` 
collapse to `mode == ALL`, so `NONE → COMMIT_TIME_ONLY` and `FILE_NAME_ONLY → 
COMMIT_TIME_AND_FILE_NAME` slipped through silently. Replaced with 
`MetaFieldsMode#isWiderThan`, which rejects any transition that *adds* a 
populated column. Narrowing stays allowed — later commits simply leave the 
column null.
   
   #### Deliberately not changed
   
   - **MoR incremental-relation guards** 
(`MergeOnReadIncrementalRelationV1/V2`) are stricter than 
`isCommitTimePopulated()` on purpose: `HoodieAppendHandle` does not yet honor 
the mode, so relaxing them would convert a loud failure into silent data loss.
   - **The ~40 cosmetic references** where `populateMetaFields()` is 
semantically identical today. Those become `isRecordKeyPopulated()` in the 
rename sweep.
   - **`HoodieTableMetadataUtil:1094/1138`** — col stats drops 
`_hoodie_commit_time` under selective modes even though it is on disk. Fixing 
it changes the MDT col-stats column set, which makes already-built partitions 
inconsistent with new ones; it needs an index-version gate and belongs in its 
own patch.
   
   ### Impact
   
   - **Storage layout**: unchanged.
   - **API**: no breakage. One new method, `MetaFieldsMode#isWiderThan`.
   - **Behavior**: the write-client guard now rejects mode transitions it 
previously allowed silently. That is the intended fix — those transitions 
produce files whose meta-column population differs across commits.
   
   ### Risk Level
   
   low
   
   Each change narrows a predicate that was resolving incorrectly for selective 
modes. Default (`ALL`) and legacy (`NONE`) behavior is unchanged, since 
`resolve()` returns the same value for both when no mode property is set.
   
   ### Documentation Update
   
   None needed — no new user-facing config.
   
   ### Contributor's checklist
   
   - [x] Read through [contributor's 
guide](https://hudi.apache.org/contribute/how-to-contribute)
   - [x] Change Logs and Impact were stated clearly
   - [x] Adequate tests were added if applicable
   - [ ] CI passed
   


-- 
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]

Reply via email to