juntaozhang commented on PR #7313:
URL: https://github.com/apache/paimon/pull/7313#issuecomment-4531574309
> Thanks for adding the Spark procedure for chain table compaction.
>
> **Comments:**
>
> 1. **PR description is minimal** — the "Purpose" section just links issue
#7312 without explaining the approach. Please describe:
>
> * How does `compact_chain_table` differ from regular `compact`?
> * Does it compact both snapshot and delta branches?
> * What's the merge strategy for chain compaction?
> 2. **File changes**: The PR touches `ChainGroupReadTable`,
`FallbackReadFileStoreTable`, and `ChainSplit`. What changes were needed to
support compaction vs. just read? Are these refactoring prerequisites or
functional changes?
> 3. **613 additions** is significant. A test file
(`CompactChainTableProcedureTest.scala`) + procedure + supporting core changes
— consider whether the core changes could be a separate prerequisite PR.
> 4. **Documentation**: Good that both `chain-table.md` and `procedures.md`
are updated.
> 5. **`ChainSplitTest`**: What cases does it cover? Chain tables have
complex split logic due to cross-branch data dependencies.
>
> Please fill in the PR description with the design approach before
requesting final review.
Thank you for the thorough review and valuable feedback.
1. PR description
Already updated in #7312.
2. Core file changes
Extracted to prerequisite PR #7950.
5. ~~ChainSplitTest / Split support~~
`ScanPlanHelper.createNewScanPlan` already supports the `Split`
interface, no additional changes needed.
--
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]
