Skip to content

HIVE-29744: Concurrent INSERT INTO on new dynamic partition causes data loss (non-ACID)#6613

Merged
difin merged 2 commits into
apache:masterfrom
difin:concurrent_insert_issue
Jul 17, 2026
Merged

HIVE-29744: Concurrent INSERT INTO on new dynamic partition causes data loss (non-ACID)#6613
difin merged 2 commits into
apache:masterfrom
difin:concurrent_insert_issue

Conversation

@difin

@difin difin commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Updated Hive.loadPartitionInternal() so INSERT INTO commits append files instead of replacing the partition directory when the partition is not yet in the metastore.

Why are the changes needed?

INSERT INTO should append data, but concurrent loads into a new dynamic partition can delete each other’s files during the MoveTask commit. That causes silent data loss with no query failure. The fix restores correct append behavior for non-ACID INSERT INTO without requiring write serialization.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Added new test classTestHiveLoadPartitionKeepExisting covering append, overwrite, and concurrent commit scenarios.

@difin
difin marked this pull request as ready for review July 16, 2026 22:48
// TODO: why is "&& !isAcidIUDoperation" needed here?
if (!isTxnTable && ((loadFileType == LoadFileType.REPLACE_ALL) || (oldPart == null && !isAcidIUDoperation))) {
if (!isTxnTable && ((loadFileType == LoadFileType.REPLACE_ALL)
|| (oldPart == null && !isAcidIUDoperation && loadFileType != LoadFileType.KEEP_EXISTING))) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why do we need isAcidIUDoperation check? isn't it already covered by isTxnTable ?

@deniskuzZ deniskuzZ Jul 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if (!isTxnTable && (loadFileType == LoadFileType.REPLACE_ALL
    || (oldPart == null && loadFileType != LoadFileType.KEEP_EXISTING))) {
...
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, it seems redundant. Removed.

@deniskuzZ deniskuzZ left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1, pending tests

@sonarqubecloud

Copy link
Copy Markdown

@difin
difin merged commit 5ae5a70 into apache:master Jul 17, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants