Skip to content

[core] Support compact for chain table#7888

Open
Stefanietry wants to merge 1 commit into
apache:masterfrom
Stefanietry:support_chain_compact
Open

[core] Support compact for chain table#7888
Stefanietry wants to merge 1 commit into
apache:masterfrom
Stefanietry:support_chain_compact

Conversation

@Stefanietry
Copy link
Copy Markdown
Contributor

@Stefanietry Stefanietry commented May 18, 2026

Purpose
Linked issue: #7887

Purpose

Support compact for chain table

Tests

org.apache.paimon.spark.SparkChainTableITCase#testChainMerge

Copy link
Copy Markdown
Contributor

@JingsongLi JingsongLi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: [core] Support compact for chain table

Critical Bug: Resource lifecycle in mapPartitions closure

In ChainMergeProcedure.java, the BatchTableWrite and IOManager are created before the while (splitIterator.hasNext()) loop, but write.close() and ioManager.close() are called inside a finally block within the loop iteration. If a Spark partition receives more than one split, the second iteration will attempt to use already-closed resources and fail. Additionally, prepareCommit() is called per-split rather than once after all data has been written.

Fix: move the try/finally to wrap the entire while-loop, and call prepareCommit() only after all splits are processed.

Hardcoded version in pom.xml

The new dependency uses 1.5-SNAPSHOT instead of ${project.version}. This will break when the project version changes.

Typo: validataChainMerge should be validateChainMerge

Inconsistent Javadoc

The class-level doc shows CALL sys.compact(...) but the actual registered procedure name is chain_merge.

Incorrect PR title prefix

Title says [core] but all changes are in paimon-spark. Should be [spark].

Test coverage suggestion

The test should also assert that the un-merged partition in the snapshot branch remains unaffected, to guard against accidental overwrites.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants