sundapeng commented on PR #10303: URL: https://github.com/apache/paimon/pull/10303#issuecomment-5971023298
Thanks for the implementation. I reviewed the latest head (`c0b5133`) and ran the targeted core/REST validation suite (395/395 passed). I did not find another deterministic issue that should block the merge. A few follow-up improvements are worth considering: 1. **P2: document the append race as well.** The append path checks the partition format before publishing files, then registers the partition and reports statistics afterward. If another client changes the partition’s `file.format` in between, Parquet files could be left with ORC metadata. The current documentation requires external serialization for overwrites, but this requirement should also cover appends and format-metadata updates, or the catalog API should eventually provide an expected-format/version conditional update. 2. Consider adding the cross-format overwrite failure-injection test to the PR. It should cover a catalog-update failure and the supported recovery path (retry or metadata repair). Simply reordering metadata and file publication would only move the inconsistency to the opposite failure path. 3. The REST API documentation and OpenAPI description should document `partitionOptions[].file.format`, including its behavior with `ignoreIfExists=true`, the difference between append and replacement statistics, and the per-batch atomicity boundary. 4. It would be useful to add a test where the physical file format and filename suffix disagree—for example, an ORC file named `00000_0` or `part-x.parquet.gz`—to make it explicit that the reader is selected from partition metadata rather than the filename. Regarding the stale `file.format` after a failed cross-format overwrite: ordinary non-ACID Hive tables have a similar non-atomic failure-recovery limitation. I would therefore treat this as a documented recovery limitation rather than a merge blocker, unless the product contract requires failed overwrites to remain automatically readable and metadata-consistent. -- 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]
