table: Non-clustered table non-unique global index needs partid in key, v1 #65380

mjonss · 2026-01-01T13:25:47Z

What problem does this PR solve?

Issue Number: close #65289

Problem Summary:
Non-clustered partitioned table's non-unique Global Index key format is not unique per index entry if _tidb_rowid is not unique across all partitons, which they may not be after EXCHANGE PARTITION.

What changed and how does it work?

Create a new version of Global Index, which also includes the partition id in the key, while still keeping the partition id in the value part of the index entry, to not extend the scope to also affect the reading part of the index (to be followed by another PR which adds yet another global index version, that only has the partition id in the key, and not in the value).

The implementation consists of two different changes:

Find out when needed, i.e. only for non-unique global indexes on non-clustered tables. This needed a bit more code, since in BuildIndexInfo we need to know if tblInfo.HasClusteredIndex(), and that might not be set correctly yet, if the primary key was not handled yet in BuildTableInfo, so we needed to do a pre-scan of the constraints to set it properly first, before actually handling all indexes
Write the partition id as part of the key (for GlobalIndexVersionV1), including fixing the index backfill function fetchRowColVals() to properly set the handle as a PartitionHandle.

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.

Side effects

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

Documentation

Release note

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

Non-clustered partitioned table's non-unique Global Index could miss entries due to duplicate _tidb_rowid across partitions, after EXCHANGE PARTITION. Starting in 8.5.4 non-unique global indexes is allowed, and those indexes can be checked by 'admin check table' and those indexes are recommended to be re-created. In 8.5.4 creating a UNIQUE GLOBAL INDEX instead is a valid workaround (easily created by adding the rest of the PK columns).

…s compatibility Add GlobalIndexVersion field to IndexInfo to enable backwards compatible evolution of global index key formats. Only non-clustered tables with non-unique global indexes use version 2, which encodes partition ID in the key using PartitionIDFlag to prevent duplicate handle collisions after EXCHANGE PARTITION. Clustered tables and unique indexes are not affected by the duplicate handle issue and continue using version 0 (legacy format). Clustered tables have unique primary keys that include the partitioning column, so no collisions are possible. Version validation ensures unsupported future versions are rejected with clear error messages. This approach allows gradual migration while prioritizing correctness for affected index types. Related to pingcap#65289. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>

ti-chi-bot · 2026-01-01T13:25:57Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign gmhdbjd for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found 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

tiprow · 2026-01-01T13:26:10Z

Hi @mjonss. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Details

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 kubernetes-sigs/prow repository.

codecov · 2026-01-01T13:47:06Z

Codecov Report

❌ Patch coverage is 92.59259% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.4342%. Comparing base (258332a) to head (c1045f7).
⚠️ Report is 18 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #65380        +/-   ##
================================================
+ Coverage   77.7820%   79.4342%   +1.6521%     
================================================
  Files          2001       1949        -52     
  Lines        545618     536275      -9343     
================================================
+ Hits         424393     425986      +1593     
+ Misses       119563     108825     -10738     
+ Partials       1662       1464       -198

Flag	Coverage Δ
integration	`47.7388% <84.2592%> (-0.4211%)`	⬇️
unit	`76.6922% <89.8148%> (+0.2727%)`	⬆️

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

Components	Coverage Δ
dumpling	`56.7974% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`66.1993% <ø> (+5.2252%)`	⬆️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

mjonss · 2026-01-01T15:32:17Z

/ok-to-test

mjonss · 2026-01-01T18:14:39Z

/test all

…entries

…MARY KEY Problem: When creating a partitioned table with an inline PRIMARY KEY (e.g., `a INT PRIMARY KEY`) followed by global indexes in the same CREATE TABLE statement, the global index was incorrectly assigned GlobalIndexVersion V1 even when the table was clustered. This happened because inline PRIMARY KEY definitions are converted to constraints and appended to the end of the constraints list (after table- level constraints). When BuildTableInfo processes constraints in order, global indexes are built before the PRIMARY KEY constraint is processed, so HasClusteredIndex() returns false, causing incorrect version assignment. Root Cause: In buildColumnsAndConstraints (create_table.go:1063), inline PRIMARY KEY constraints from column definitions are appended after table constraints: constraints = append(constraints, cts...) So for `CREATE TABLE t (a INT PRIMARY KEY, key idx(b) global)`, constraints are processed as: 1. key idx(b) global <- built first, sees HasClusteredIndex()=false 2. primary key(a) <- processed last, sets PKIsHandle=true Solution: Added a pre-scan loop before constraint processing in BuildTableInfo that: 1. Scans all constraints to find PRIMARY KEY 2. Determines if table will be clustered (PKIsHandle/IsCommonHandle) 3. Sets clustering flags before any indexes are built This ensures HasClusteredIndex() returns correct value when building global indexes, so: - Clustered tables get GlobalIndexVersion = 0 (correct) - Non-clustered tables with non-unique global indexes get V1 (correct) The pre-scan handles expression-based PKs correctly without building hidden columns (expression PKs can't be PKIsHandle, so they become IsCommonHandle). Impact: - Preserves constraint ordering (no SHOW CREATE TABLE changes) - Fixes TestGlobalIndexVersion1 which was failing - No performance impact (lightweight pre-scan) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

pkg/ddl/tests/partition/global_index_version_test.go

…lustered-non-unique-add-partid-to-key-65289

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

pkg/ddl/create_table.go:1409

The clustering index information (PKIsHandle, IsCommonHandle, CommonHandleVersion) is being set twice - once in the pre-scan (lines 1332-1348) and again in the main loop (lines 1403-1409). This duplication could lead to maintenance issues. While this appears intentional to ensure the information is available early for BuildIndexInfo calls, the duplicate logic increases the risk of inconsistencies if one location is updated without the other. Consider adding a comment in the main loop referencing the pre-scan or refactoring to eliminate the duplication by extracting the logic into a shared function.

			if ShouldBuildClusteredIndex(ctx.GetClusteredIndexDefMode(), constr.Option, isSingleIntPK) {
				if isSingleIntPK {
					tbInfo.PKIsHandle = true
				} else {
					tbInfo.IsCommonHandle = true
					tbInfo.CommonHandleVersion = 1
				}

pkg/tablecodec/tablecodec.go

pkg/table/tables/index.go

mjonss · 2026-01-22T18:26:37Z

/retest

mjonss · 2026-01-23T10:54:10Z

/retest

Co-authored-by: Copilot <[email protected]>

…lustered-non-unique-add-partid-to-key-65289

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

pkg/tablecodec/tablecodec.go

mjonss · 2026-01-23T15:55:36Z

/retest

…lustered-non-unique-add-partid-to-key-65289

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

pkg/ddl/index.go

pkg/ddl/partition.go

…lustered-non-unique-add-partid-to-key-65289

mjonss · 2026-01-30T15:17:50Z

/retest

mjonss and others added 3 commits December 29, 2025 11:14

table: Global index did not correctly use partID+_tidb_rowid

30b1f12

bazel_prepare

e102f6a

ti-chi-bot bot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jan 1, 2026

ti-chi-bot bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jan 1, 2026

Cleanup

fd722bb

ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 1, 2026

Linting

6059bde

mjonss marked this pull request as draft January 1, 2026 15:02

ti-chi-bot bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 1, 2026

mjonss added 2 commits January 1, 2026 15:02

Linting

2e01584

Removed debug code that failed linting

f13baf2

ti-chi-bot bot added the ok-to-test Indicates a PR is ready to be tested. label Jan 1, 2026

Cleanup

dd33176

mjonss added 2 commits January 1, 2026 19:12

Added test for old version global index

3b824df

Fixed case where INSERT did not use PartitionHandle for adding index …

7013ec4

…entries

mjonss marked this pull request as ready for review January 1, 2026 21:03

ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 1, 2026

mjonss and others added 5 commits January 1, 2026 22:28

Updated test and fixed found issue

ee50be1

Fixed issue with clustered IntHandle tables

ef6420f

temp commit for trouble-shooting how to check if Clustered or not...

cac17ca

cleanup of debug in test

c175fcb

mjonss requested a review from Copilot January 2, 2026 00:51

mjonss added 3 commits January 15, 2026 23:08

Removed non-needed change

d3d17e0

Removed more non-needed changes

ea589be

Added back a comment that should not have been removed...

edc40d8

Copilot AI review requested due to automatic review settings January 19, 2026 11:48

Copilot started reviewing on behalf of mjonss January 19, 2026 11:48 View session

Copilot AI reviewed Jan 19, 2026

View reviewed changes

pkg/ddl/tests/partition/global_index_version_test.go Show resolved Hide resolved

pkg/ddl/tests/partition/global_index_version_test.go Outdated Show resolved Hide resolved

pkg/ddl/tests/partition/global_index_version_test.go Show resolved Hide resolved

mjonss added 2 commits January 21, 2026 10:42

Merge remote-tracking branch 'pingcap/master' into global-index-non-c…

bfcc751

…lustered-non-unique-add-partid-to-key-65289

Added tests for copilots review comments

c4c252d

Copilot AI review requested due to automatic review settings January 21, 2026 11:13

Copilot started reviewing on behalf of mjonss January 21, 2026 11:13 View session

Copilot AI reviewed Jan 21, 2026

View reviewed changes

pkg/tablecodec/tablecodec.go Outdated Show resolved Hide resolved

pkg/table/tables/index.go Outdated Show resolved Hide resolved

Apply suggestions from code review

27c42a2

Co-authored-by: Copilot <[email protected]>

Copilot AI review requested due to automatic review settings January 23, 2026 15:39

Copilot started reviewing on behalf of mjonss January 23, 2026 15:40 View session

Merge remote-tracking branch 'pingcap/master' into global-index-non-c…

0473d15

…lustered-non-unique-add-partid-to-key-65289

Copilot AI reviewed Jan 23, 2026

View reviewed changes

pkg/tablecodec/tablecodec.go Show resolved Hide resolved

mjonss added 3 commits January 23, 2026 17:26

Fixed issue with nested PartitionHandle

d5c43fa

bazel_prepare

fc2ce59

Merge remote-tracking branch 'pingcap/master' into global-index-non-c…

d7a3a30

…lustered-non-unique-add-partid-to-key-65289

Copilot AI review requested due to automatic review settings January 26, 2026 09:37

Copilot started reviewing on behalf of mjonss January 26, 2026 09:38 View session

Copilot AI reviewed Jan 26, 2026

View reviewed changes

pkg/ddl/index.go Outdated Show resolved Hide resolved

pkg/ddl/partition.go Outdated Show resolved Hide resolved

pkg/ddl/partition.go Outdated Show resolved Hide resolved

mjonss added 2 commits January 30, 2026 14:26

Fix overwritten global index entries for unique null keys

b1565a9

Merge remote-tracking branch 'pingcap/master' into global-index-non-c…

cb95d58

…lustered-non-unique-add-partid-to-key-65289

mjonss added 2 commits February 3, 2026 11:53

Test fix

34df699

Removed using subtests in tablecodec_test

c1045f7

table: Non-clustered table non-unique global index needs partid in key, v1 #65380

Are you sure you want to change the base?

table: Non-clustered table non-unique global index needs partid in key, v1 #65380

Uh oh!

Conversation

mjonss commented Jan 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

Uh oh!

ti-chi-bot bot commented Jan 1, 2026

Uh oh!

tiprow bot commented Jan 1, 2026

Uh oh!

codecov bot commented Jan 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mjonss commented Jan 1, 2026

Uh oh!

mjonss commented Jan 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

mjonss commented Jan 22, 2026

Uh oh!

mjonss commented Jan 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

mjonss commented Jan 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mjonss commented Jan 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mjonss commented Jan 1, 2026 •

edited

Loading

codecov bot commented Jan 1, 2026 •

edited

Loading