Skip to content

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

Merged
ti-chi-bot[bot] merged 2 commits into
pingcap:release-nextgen-202603from
ti-chi-bot:cherry-pick-70374-to-release-nextgen-202603
Aug 7, 2026
Merged

ti-chi-bot[bot] merged 2 commits into
pingcap:release-nextgen-202603from
ti-chi-bot:cherry-pick-70374-to-release-nextgen-202603

Conversation

@ti-chi-bot

@ti-chi-bot ti-chi-bot commented Aug 7, 2026 •

Copy link
Copy Markdown
Member

This is an automated cherry-pick of #70374

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
    • Improved conflict handling for tables with functional indexes after visible columns are added.
    • Ensured conflict records preserve all visible row data and associated index entries.
    • Fixed scenarios where duplicate functional-index rows could be processed incorrectly during import.
    • Confirmed valid rows can still be inserted successfully after conflicts are detected.
    • Improved reliability of conflict reporting and table validation following duplicate-row imports.

@ti-chi-bot ti-chi-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. type/cherry-pick-for-release-nextgen-202603 labels Aug 7, 2026
@coderabbitai

coderabbitai Bot commented Aug 7, 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: 2de9d9b2-34f8-45f3-b1af-af27741cb914

📥 Commits

Reviewing files that changed from the base of the PR and between 4527a80 and 0c62513.

📒 Files selected for processing (3)
  • pkg/bindinfo/tests/BUILD.bazel
  • pkg/dxf/importinto/conflictedkv/handler_test.go
  • pkg/util/stmtsummary/BUILD.bazel
🚧 Files skipped from review as they are similar to previous changes (1)
  • pkg/dxf/importinto/conflictedkv/handler_test.go

📝 Walkthrough

Walkthrough

Conflict cleanup now decodes only visible columns before re-encoding rows. Unit and NextGen integration tests cover functional-index conflicts after visible-column and unique-index additions. Two Bazel test targets also use lower shard counts.

Changes

Conflict cleanup

Layer / File(s) Summary
Visible-column conflict re-encoding
pkg/dxf/importinto/conflictedkv/handler.go, pkg/dxf/importinto/conflictedkv/handler_test.go
encodeAndHandleRow decodes visible columns before re-encoding conflict rows. Unit coverage checks record, functional-index, and tail index KVs.
Schema-evolution integration regression
tests/realtikvtest/importintotest4/conflict_resolution_test.go
The NextGen test imports duplicate functional-index rows with conflict capture, verifies two conflicts and an empty table, then validates a later insert and table consistency.

Test sharding

Layer / File(s) Summary
Bazel test shard configuration
pkg/bindinfo/tests/BUILD.bazel, pkg/util/stmtsummary/BUILD.bazel
The test targets reduce their shard counts from 26 to 24 and from 33 to 30.

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

Possibly related PRs

  • pingcap/tidb#70076: Updates the same conflict-KV handler and visible-column regression scenario.
  • pingcap/tidb#70208: Extends related encodeAndHandleRow and functional-index conflict coverage.
  • pingcap/tidb#70374: Covers the same functional-index conflict cleanup issue after schema evolution.

Suggested labels: cherry-pick-approved, ok-to-test

Suggested reviewers: joechenrh, wjhuang2016

Poem

I’m a rabbit guarding each KV,
Visible columns guide me true.
Conflicts fade, indexes align,
A later insert works fine.
Hop, hop—clean tables shine!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR also reduces Bazel shard counts for unrelated bindinfo and statement-summary tests, which are outside the linked issue scope. Remove the unrelated Bazel shard-count changes, or link a separate issue that explicitly requires those test-target adjustments.
✅ Passed checks (4 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 issue, root cause, implementation, tests, side effects, documentation impact, and release note.
Linked Issues check ✅ Passed The code changes address the linked issue by decoding visible columns and adding regressions for cleanup, consistency, checksums, and uniqueness.
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

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.

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

D3Hunter commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

/retest

@ti-chi-bot ti-chi-bot Bot added sig/planner SIG: Planner and removed approved labels Aug 7, 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: D3Hunter, qw4990

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 approved 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-07 08:12:49.172422935 +0000 UTC m=+2774955.208517981: ☑️ agreed by D3Hunter.
  • 2026-08-07 09:28:47.230621256 +0000 UTC m=+2779513.266716302: ☑️ agreed by qw4990.

@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (release-nextgen-202603@6b4c9f9). Learn more about missing BASE report.

Additional details and impacted files
@@                     Coverage Diff                     @@
##             release-nextgen-202603     #70385   +/-   ##
===========================================================
  Coverage                          ?   76.2468%           
===========================================================
  Files                             ?       1937           
  Lines                             ?     541473           
  Branches                          ?          0           
===========================================================
  Hits                              ?     412856           
  Misses                            ?     128617           
  Partials                          ?          0           
Flag Coverage Δ
unit 76.2468% <100.0000%> (?)

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

Components Coverage Δ
dumpling 61.5065% <0.0000%> (?)
parser ∅ <0.0000%> (?)
br 48.7938% <0.0000%> (?)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ti-chi-bot
ti-chi-bot Bot merged commit ae13335 into pingcap:release-nextgen-202603 Aug 7, 2026
18 checks passed
@ti-chi-bot
ti-chi-bot Bot deleted the cherry-pick-70374-to-release-nextgen-202603 branch August 7, 2026 10:01
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. sig/planner SIG: Planner size/L Denotes a PR that changes 100-499 lines, ignoring generated files. type/cherry-pick-for-release-nextgen-202603

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants