yihua commented on issue #375: URL: https://github.com/apache/hudi-rs/issues/375#issuecomment-6699414404
Thanks @oglego, this looks like the right direction! Checking in `OptionResolver` before the merge and comparing parsed values both make sense to me. A few things I noticed while testing it against main: 1. Could we loop over every key in `hoodie.properties` rather than only the enum? Persisted keys like `hoodie.record.merge.mode` affect reads and would still be silently replaced. Enum keys can compare parsed, and the rest can compare raw. 2. Aliases need a check too: `hoodie.table.ordering.fields` on a table that persists `hoodie.table.precombine.field` currently wins over the table value. Running `parse_value` on separate user and table maps should catch it, and it would be great to have a test. 3. Most table configs are immutable in `hoodie.properties`, except for metadata related configs, so `MetadataTablePartitions` and `MetadataTableEnabled` shouldn't raise a conflict. I'd also leave out `BasePath` (the resolver sets it), `Checksum` (derived) and `CreateSchema` (JSON formatting would cause false conflicts), and skip `DatabaseName` when the table's value is empty. 4. `write/rewrite/compaction.rs` reopens the table with `table.hudi_options()`, so resolved values come back in as user options. Could that reopen take a separate path, so a later `hoodie.properties` change doesn't fail background compaction? Two small things: it would help to report all conflicts in sorted order through a new `ConfigError` variant, and to compare against a snapshot taken before the merge, since `build()` can be called twice on one builder. Looking forward to the PR! -- 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]
