JingsongLi commented on PR #8616: URL: https://github.com/apache/paimon/pull/8616#issuecomment-5748062439
I reviewed the current head as an end-to-end clone/migration feature. **Requirement fit: NEEDS-EVIDENCE. Code review: FINDINGS.** Preserving full table history can be valuable for backup or migration, but this PR still does not establish the production workflow requested earlier. At roughly 9k added lines, it introduces a large new protocol while explicitly requiring the source to stop, excluding BLOB/Iceberg state, and leaving target-catalog registration outside the operation. A `_SUCCESS` marker alone does not make the cloned table discoverable or demonstrate an operationally complete migration. Before this can be accepted, please provide one concrete production use case and an end-to-end runbook covering source quiescing, target catalog registration, retry/resume behavior, validation, and rollback. The change should also be split into independently reviewable pieces (file-set/planning, copy/retry, metadata rewrite/validation, and engine/procedure integration). The current head is also not green: multiple build jobs fail, including `FullHistoryMetadataRewriterTest.testRewriteAllHistoryWithExternalDataPaths`, where `listAllIds()` returned `[1, 0]` but the test requires `[0, 1]`. If ordering is not part of the schema-manager contract, make the assertion order-independent; if ordering is required, fix the implementation and document that contract. I am not closing this because the capability can have end-to-end value, but the current PR needs production evidence, a smaller review surface, and green deterministic tests before the implementation can be meaningfully approved. -- 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]
