Skip to content

import into: fix conflict cleanup after functional-index schema evolution - #70374

Merged
ti-chi-bot[bot] merged 1 commit into
pingcap:masterfrom
D3Hunter:codex/import-into-visible-cols-conflict-cleanup
Aug 7, 2026
Merged

ti-chi-bot[bot] merged 1 commit into
pingcap:masterfrom
D3Hunter:codex/import-into-visible-cols-conflict-cleanup

Conversation

@D3Hunter

@D3Hunter D3Hunter commented Aug 6, 2026 •

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: close #70372

Problem Summary:

When IMPORT INTO ... WITH on_duplicate_key='capture' resolves conflicts for a table whose functional-index hidden column physically precedes a later visible column, conflict cleanup decodes persisted rows using all table columns but re-encodes them as visible-column input. The positional mismatch can reconstruct and delete the wrong ordinary index keys. The import can then fail checksum validation after leaving ghost unique-index entries or removing live uniqueness protection.

What changed and how does it work?

  • Decode persisted conflict rows with VisibleCols() so the decoded datum slice uses the same positional domain as the duplicate-resolution encoder.
  • Keep hidden functional-index columns out of parser-visible input; the existing table encoder continues to recompute their generated values.
  • Add a handler regression that verifies the complete reconstructed KV set and the ordinary unique index added after a functional index.
  • Add a NextGen RealTiKV regression covering global-sort import, conflict collection and deletion, checksum validation, ADMIN CHECK TABLE, and subsequent uniqueness enforcement.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No need to test
    • I checked and no code files have been changed.

Validation performed:

  • ./tools/check/failpoint-go-test.sh pkg/dxf/importinto/conflictedkv -run '^TestHandler$' -count=1
  • NEXT_GEN=1 go test -count=1 -v -run '^TestImportInto$/^TestGlobalSortFunctionalIndexConflictAfterAddingVisibleColumn$' -tags=intest,nextgen -timeout=40m ./tests/realtikvtest/importintotest4 -args -with-real-tikv -tikv-path 'tikv://127.0.0.1:2379?disableGC=true'
  • make lint
  • Manual reproduction on a NextGen cluster with MinIO, PD, three TiKV stores, TiKV Worker, SYSTEM TiDB, and a user-keyspace TiDB. The fixed import finished with two captured conflicts, both ADMIN CHECK TABLE statements passed, and inserting a row using the formerly ghosted unique value succeeded.

The RealTiKV regression was also run against the old Cols() behavior and reproduced the checksum mismatch (total_kvs: 2 vs 0, total_bytes: 80 vs 21). The identical test passed with VisibleCols().

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

Fix an issue where `IMPORT INTO` conflict cleanup could leave corrupt unique-index entries after adding visible columns to a table with a functional index.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed conflict handling during imports for tables with functional indexes and newly added visible or unique columns.
    • Ensured row data retains all expected key-value pairs and visible column values.
    • Prevented invalid duplicate functional-index rows from being imported.
  • Tests

    • Added regression coverage for import conflict resolution and data consistency.

@ti-chi-bot ti-chi-bot Bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 6, 2026
@coderabbitai

coderabbitai Bot commented Aug 6, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4c894f3e-b185-4d40-90d6-f51811213fdf

📥 Commits

Reviewing files that changed from the base of the PR and between 3a5407d and 9417073.

📒 Files selected for processing (3)
  • pkg/dxf/importinto/conflictedkv/handler.go
  • pkg/dxf/importinto/conflictedkv/handler_test.go
  • tests/realtikvtest/importintotest4/conflict_resolution_test.go

📝 Walkthrough

Walkthrough

The conflict handler now decodes rows with visible columns before re-encoding cleanup KVs. Unit and NextGen integration tests cover functional-index schema evolution, captured conflicts, table consistency, and subsequent inserts.

Changes

Conflict cleanup correction

Layer / File(s) Summary
Visible-column decoding and handler validation
pkg/dxf/importinto/conflictedkv/handler.go, pkg/dxf/importinto/conflictedkv/handler_test.go
encodeAndHandleRow decodes with VisibleCols(). Tests verify preservation of record and index KVs, including the tail index.
NextGen conflict-resolution regression
tests/realtikvtest/importintotest4/conflict_resolution_test.go
The regression test covers duplicate functional-index rows after adding a visible column and unique index. It verifies captured conflicts, table consistency, and a valid later insert.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested labels: component/import

Suggested reviewers: joechenrh, wjhuang2016

Poem

A rabbit checked each index key,
And found the hidden mismatch flee.
Visible columns now guide the way,
While clean conflicts leave rows okay.
Hop, hop—valid inserts stay!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the IMPORT INTO conflict-cleanup fix for functional-index schema evolution.
Description check ✅ Passed The description includes the required issue reference, problem, implementation, tests, side effects, documentation, and release note.
Linked Issues check ✅ Passed The change addresses issue #70372 by aligning conflict-row decoding with visible-column encoding and adding targeted regression coverage.
Out of Scope Changes check ✅ Passed All changes are in scope: the production fix and unit and integration regressions directly support issue #70372.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="Running error: context loading failed: failed to load packages: failed to load packages: failed to load with go/packages: context deadline exceeded"
level=error msg="Timeout exceeded: try increasing it by passing --timeout option"


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@D3Hunter

D3Hunter commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

/cherry-pick release-nextgen-202603

@ti-chi-bot

Copy link
Copy Markdown
Member

@D3Hunter: once the present PR merges, I will cherry-pick it on top of release-nextgen-202603 in the new PR and assign it to you.

Details

In response to this:

/cherry-pick release-nextgen-202603

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

@D3Hunter

D3Hunter commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

/cherry-pick release-nextgen-202603

@ti-chi-bot

Copy link
Copy Markdown
Member

@D3Hunter: once the present PR merges, I will cherry-pick it on top of release-nextgen-202603 in the new PR and assign it to you.

Details

In response to this:

/cherry-pick release-nextgen-202603

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

@codecov

codecov Bot commented Aug 6, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 73.4518%. Comparing base (bff1263) to head (9417073).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@               Coverage Diff                @@
##             master     #70374        +/-   ##
================================================
- Coverage   76.3261%   73.4518%   -2.8743%     
================================================
  Files          2041       2079        +38     
  Lines        558970     583823     +24853     
================================================
+ Hits         426640     428829      +2189     
- Misses       131430     154488     +23058     
+ Partials        900        506       -394     
Flag Coverage Δ
integration 40.8070% <0.0000%> (+1.1383%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
dumpling 59.9240% <ø> (ø)
parser ∅ <ø> (∅)
br 46.6167% <ø> (-16.0923%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ingress-bot

Copy link
Copy Markdown

🔍 Starting code review for this PR...

@ingress-bot ingress-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This review was generated by AI and should be verified by a human reviewer.
Manual follow-up is recommended before merge.

Summary

  • Total findings: 0
  • Inline comments: 0
  • Summary-only findings (no inline anchor): 0
Findings (highest risk first)

No findings.

@ti-chi-bot ti-chi-bot Bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Aug 6, 2026
@ti-chi-bot

ti-chi-bot Bot commented Aug 7, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: GMHDBJD, wjhuang2016

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot Bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Aug 7, 2026
@ti-chi-bot

ti-chi-bot Bot commented Aug 7, 2026

Copy link
Copy Markdown

[LGTM Timeline notifier]

Timeline:

  • 2026-08-06 13:25:42.137070534 +0000 UTC m=+2707328.173165580: ☑️ agreed by wjhuang2016.
  • 2026-08-07 07:18:24.605314049 +0000 UTC m=+2771690.641409105: ☑️ agreed by GMHDBJD.

@ti-chi-bot
ti-chi-bot Bot merged commit f2e2965 into pingcap:master Aug 7, 2026
34 checks passed
@ti-chi-bot

Copy link
Copy Markdown
Member

@D3Hunter: new pull request created to branch release-nextgen-202603: #70385.

Details

In response to this:

/cherry-pick release-nextgen-202603

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

@D3Hunter
D3Hunter deleted the codex/import-into-visible-cols-conflict-cleanup branch August 7, 2026 08:07
ti-chi-bot Bot pushed a commit that referenced this pull request Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

IMPORT INTO conflict cleanup can persist corrupt unique indexes after functional-index schema evolution

5 participants