SEPURI-SAI-KRISHNA commented on issue #19669:
URL: https://github.com/apache/hudi/issues/19669#issuecomment-5371593361
I'm working on this one. Before I put up a patch, I would like to settle how
hard the check should
fail, because the obvious place to put it has a compatibility wrinkle.
The fix as filed is to move the mutual-exclusion check into
`HoodieWriterUtils.validateTableConfig`
so it covers `df.write` and HoodieStreamer, not just SQL DDL. That method
runs on **every**
non-overwrite write, comparing incoming params against the existing table
config — it is skipped
only when the save mode is `Overwrite`. So a check added there does not just
reject new tables: it
starts throwing on the next write to any table that already has both configs
set. Those tables are
in a bad state today (the Avro and row-writer paths disagree on the
directory, and the Avro layout
cannot be read back by `HoodieSparkUtils#doParsePartitionColumnValues`), but
their writes currently
succeed, so this would turn a silent problem into a hard failure on upgrade.
Options, roughly in increasing order of strictness:
1. **Reject only at table creation**, leaving existing tables writable.
Matches what
`HoodieCatalogTable#extraTableConfig` already does for SQL, just extended
to the other write
paths. Least disruptive, but leaves already-broken tables silently broken.
2. **Warn on every write, reject at creation.** Same as 1 plus a loud log
line so operators of
existing tables find out.
3. **Reject on every write**, i.e. the straightforward `validateTableConfig`
check. Most correct,
but breaks existing pipelines on upgrade with no migration path, since
the data already on disk
is in whichever layout the writer produced.
I lean towards 2: the combination is documented as unsupported, so silently
accepting it is the
actual bug, but failing an established pipeline mid-flight seems worse than
telling the operator
loudly. Which would you prefer? Happy to implement whichever the maintainers
want.
Note this interacts with #19666: if multi-field slash partitioning is also
rejected at write time,
both checks land in the same method and should fail the same way.
--
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]
