Skip to content

fix(replace): handle one new row conflicting with multiple old rows via different unique keys#24581

Merged
mergify[bot] merged 3 commits into
matrixorigin:mainfrom
ck89119:issue-24428
May 27, 2026
Merged

fix(replace): handle one new row conflicting with multiple old rows via different unique keys#24581
mergify[bot] merged 3 commits into
matrixorigin:mainfrom
ck89119:issue-24428

Conversation

@ck89119
Copy link
Copy Markdown
Contributor

@ck89119 ck89119 commented May 25, 2026

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

issue #24428

What this PR does / why we need it:

REPLACE INTO produced a duplicate-key error when a single new row conflicted with
multiple existing rows via different unique keys. The expected MySQL semantics are:
delete all conflicting old rows, then insert the new row exactly once.

This fixes two cases:

  1. Real PK + multiple unique keys (pkg/sql/colexec/hashbuild/hashmap.go)
    The OR'd LEFT JOIN fans one new row into several build rows that carry different
    old PKs. keep-last keeps one of them and converts the rest into delete-only rows,
    but their old-PK column was nulled and the delColIdx delete-marker pass only
    scanned the surviving rows. So the surviving bucket was not marked deleted, and when
    the new PK also matched an existing row the dedup-join probe raised a false
    Duplicate entry. Fix: keep the old-PK column on the delete-only rows and let the
    delColIdx pass scan them, so the surviving bucket is correctly marked deleted.

  2. Fake PK + multiple unique keys (pkg/sql/plan/bind_replace.go)
    Fake-PK tables built the conflict-detection LEFT JOIN from only the first unique
    key, missing conflicts on the others. Fix: OR one (AND-of-parts) condition per unique
    key, matching the real-PK path.

The single-UK fix (#24425) and the duplicate-source-key keep-last behavior (#24497) are
preserved.

How verified

  • New hashbuild unit test TestDedupBuildKeepLastMarksConflictBucketForDiscardedFanout
    (fails without the fix, passes with it).
  • New BVT cases in test/distributed/cases/dml/replace/replace.{test,result} covering
    multi-UK / explicit-PK + multi-UK / fake-PK / composite-UK / multi-row fan-out.
  • Verified end-to-end on a local mo-service; existing replace / dedupjoin / hashbuild /
    multi_update unit tests pass; make static-check is clean.

🤖 Generated with Claude Code

…ia different unique keys

REPLACE INTO raised a duplicate-key error when a single new row conflicted with
several existing rows through different unique keys (issue matrixorigin#24428). The expected
MySQL semantics are: delete all conflicting old rows, then insert the new row once.

Root causes and fixes:

1. hashbuild (real PK + multi-UK): the OR'd LEFT JOIN fans one new row into
   several build rows carrying different old PKs. keep-last keeps one and turns
   the rest into delete-only rows, but their old-PK column was nulled and the
   delColIdx delete-marker pass only scanned surviving rows, so the surviving
   bucket was not marked deleted. When the new PK also matched an existing row,
   the dedup-join probe then raised a false DuplicateEntry. Preserve the old-PK
   column on delete-only rows and scan them in the delColIdx pass.

2. bind_replace (fake PK): fake-PK tables built the conflict-detection LEFT JOIN
   from only the first unique key, missing conflicts on the others. OR one
   condition per unique key, matching the real-PK path.

Adds a hashbuild unit test for the discarded-fan-out delete marking and BVT
coverage for multi-UK / explicit-PK+multi-UK / fake-PK / composite-UK fan-out.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@qodo-code-review
Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

Copy link
Copy Markdown
Contributor

@aunjgr aunjgr left a comment

Choose a reason for hiding this comment

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

No blocking issues found.

This follow-up completes the REPLACE multi-unique-key fan-out path: it preserves the old-PK value on delete-only rows, scans the full build batch when marking delete buckets, and adds unit/BVT coverage for the previously failing fan-out cases.

Copy link
Copy Markdown
Contributor

@XuPeng-SH XuPeng-SH left a comment

Choose a reason for hiding this comment

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

No blocking issues found.

I checked the multi-UK fan-out fix from a few angles: the fake-PK LEFT JOIN now matches conflicts on any unique key, the hashbuild keep-last path now preserves/scans the old-PK values needed to mark surviving conflict buckets as deleted, and the new unit/BVT coverage matches the previously failing REPLACE cases. The change looks consistent with the existing delete-only / DelRows / dedup-join flow.

@mergify mergify Bot added the queued label May 27, 2026
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented May 27, 2026

Merge Queue Status

  • Entered queue2026-05-27 04:09 UTC · Rule: main
  • Checks started · in-place
  • Checks failed
  • 🚫 Left the queue2026-05-27 04:56 UTC · at 041fd9246d89e89c19c19eab3cff46ac8f10b56c

This pull request spent 47 minutes 24 seconds in the queue, with no time running CI.

Waiting for
  • any of: [🛡 GitHub branch protection]
    • check-neutral = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-skipped = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-success = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
  • any of: [🛡 GitHub branch protection]
    • check-neutral = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-skipped = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-success = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
  • any of: [🛡 GitHub branch protection]
    • check-neutral = Matrixone CI / UT Test on Ubuntu/x86
    • check-skipped = Matrixone CI / UT Test on Ubuntu/x86
    • check-success = Matrixone CI / UT Test on Ubuntu/x86
  • any of: [🛡 GitHub branch protection]
    • check-neutral = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
    • check-skipped = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
    • check-success = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
  • any of: [🛡 GitHub branch protection]
    • check-neutral = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
    • check-skipped = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
    • check-success = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
  • any of: [🛡 GitHub branch protection]
    • check-neutral = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
    • check-skipped = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
    • check-success = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
  • any of: [🛡 GitHub branch protection]
    • check-neutral = Matrixone Utils CI / Coverage
    • check-skipped = Matrixone Utils CI / Coverage
    • check-success = Matrixone Utils CI / Coverage
All conditions
  • any of [🛡 GitHub branch protection]:
    • check-neutral = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-skipped = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-success = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
  • any of [🛡 GitHub branch protection]:
    • check-neutral = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-skipped = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-success = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
  • any of [🛡 GitHub branch protection]:
    • check-neutral = Matrixone CI / UT Test on Ubuntu/x86
    • check-skipped = Matrixone CI / UT Test on Ubuntu/x86
    • check-success = Matrixone CI / UT Test on Ubuntu/x86
  • any of [🛡 GitHub branch protection]:
    • check-neutral = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
    • check-skipped = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
    • check-success = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
  • any of [🛡 GitHub branch protection]:
    • check-neutral = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
    • check-skipped = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
    • check-success = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
  • any of [🛡 GitHub branch protection]:
    • check-neutral = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
    • check-skipped = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
    • check-success = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
  • any of [🛡 GitHub branch protection]:
    • check-neutral = Matrixone Utils CI / Coverage
    • check-skipped = Matrixone Utils CI / Coverage
    • check-success = Matrixone Utils CI / Coverage
  • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
  • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • github-review-decision = APPROVED [🛡 GitHub branch protection]
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
    • check-neutral = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
    • check-skipped = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone CI / SCA Test on Ubuntu/x86
    • check-neutral = Matrixone CI / SCA Test on Ubuntu/x86
    • check-skipped = Matrixone CI / SCA Test on Ubuntu/x86

Reason

The merge conditions cannot be satisfied due to failing checks

Hint

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio queue comment.

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented May 27, 2026

Merge Queue Status

  • Entered queue2026-05-27 08:08 UTC · Rule: main
  • Checks passed · in-place
  • Merged2026-05-27 09:14 UTC · at 0eab34ae2fd6dd9b4d6ce17abb8dd0fe67fde582 · squash

This pull request spent 1 hour 6 minutes 23 seconds in the queue, including 1 hour 5 minutes 32 seconds running CI.

Required conditions to merge
  • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
  • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • github-review-decision = APPROVED [🛡 GitHub branch protection]
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
    • check-neutral = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
    • check-skipped = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-neutral = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-skipped = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-neutral = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-skipped = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone CI / SCA Test on Ubuntu/x86
    • check-neutral = Matrixone CI / SCA Test on Ubuntu/x86
    • check-skipped = Matrixone CI / SCA Test on Ubuntu/x86
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone CI / UT Test on Ubuntu/x86
    • check-neutral = Matrixone CI / UT Test on Ubuntu/x86
    • check-skipped = Matrixone CI / UT Test on Ubuntu/x86
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
    • check-neutral = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
    • check-skipped = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
    • check-neutral = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
    • check-skipped = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
    • check-neutral = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
    • check-skipped = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Utils CI / Coverage
    • check-neutral = Matrixone Utils CI / Coverage
    • check-skipped = Matrixone Utils CI / Coverage

@mergify mergify Bot removed the dequeued label May 27, 2026
@mergify mergify Bot merged commit bd86141 into matrixorigin:main May 27, 2026
23 of 24 checks passed
@mergify mergify Bot removed the queued label May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Something isn't working size/M Denotes a PR that changes [100,499] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants