nsivabalan commented on PR #19205:
URL: https://github.com/apache/hudi/pull/19205#issuecomment-4961137371

   Addressed the round of self-review comments in 
[cfd1fea706ed](https://github.com/apache/hudi/pull/19205/commits/cfd1fea706edee5d245201f057cd8755e724d9af)
 (branch now rebased on latest upstream/master, force-pushed):
   
   1. **Fold `populateMetaFields` + `metaFieldsMode` into one argument** — 
done. Introduced `MetaFieldsMode` enum (`ALL` / `NONE` / `COMMIT_TIME_ONLY` / 
`FILE_NAME_ONLY` / `COMMIT_TIME_AND_FILE_NAME`) as the sole meta-field argument 
through every writer factory / constructor. `populate.meta.fields` stays on the 
config surface for backward compat and is auto-derived. On-disk representation 
is now the enum name.
   2. **Drop `_hoodie_commit_seqno` from selective path** — done in 
`HoodieRowCreateHandle`, `HoodieSparkParquetWriter`, `HoodieAvroParquetWriter`. 
Only `_hoodie_commit_time` is populated when opted in.
   3. **Non-reversibility message on `IncrementalRelation`** — done: extended 
message states the mode is physical-storage, cannot be flipped by write 
options, table must be recreated. The follow-up CLI PR 
[#19206](https://github.com/apache/hudi/pull/19206) adds `table 
set-meta-fields-mode` as the sanctioned mutation path with `--force` guarding 
tables that already have commits.
   
   **Re: "can we confirm the mode is immutable at runtime + tests across 
HoodieStreamer / spark datasource"**
   
   - **Runtime-immutability guard**: `HoodieWriterUtils.validateTableConfig` 
now blocks both `null → non-empty` and `non-empty → different` transitions on 
`hoodie.meta.fields.mode`, with an error message pointing at the CLI / upgrade 
path (previously the loop only flagged the mismatch when the on-disk value was 
non-null, which let a silent-drop path slip through for older tables).
   - **HoodieStreamer coverage**: new `TestHoodieStreamerMetaFieldsMode` under 
`hudi-utilities/src/test/java/.../deltastreamer/` — parameterized end-to-end 
test through the `HoodieStreamer` entrypoint covering all 5 modes plus the 
MoR+selective rejection assertion.
   - **Spark datasource coverage**: new `TestMetaFieldsMode` under 
`hudi-spark-datasource/hudi-spark/src/test/java/.../functional/` — end-to-end 
assertions that read the written parquet back and verify which meta columns are 
populated / null per mode. Covers row-writer path (bulk_insert with 
`row.writer.enable=true`, default) AND non-row-writer path (insert with 
`row.writer.enable=false`). Also adds inline-clustering coverage for all 5 
modes verifying that clustered files preserve the mode.


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