ddl: supports non-unique global index #58678

Defined2014 · 2025-01-03T06:25:16Z

What problem does this PR solve?

Issue Number: close #58650

Problem Summary: Supports non-unqiue global index. Not found bugs right now, we will add more test cases later.

What changed and how does it work?

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.

None

tiprow · 2025-01-03T06:25:33Z

Hi @Defined2014. 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.

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 · 2025-01-03T06:41:30Z

Codecov Report

Attention: Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.

Project coverage is 75.0823%. Comparing base (4721bc3) to head (0c5529c).
Report is 25 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #58678        +/-   ##
================================================
+ Coverage   73.0940%   75.0823%   +1.9882%     
================================================
  Files          1677       1723        +46     
  Lines        463715     482854     +19139     
================================================
+ Hits         338948     362538     +23590     
+ Misses       103902      97445      -6457     
- Partials      20865      22871      +2006

Flag	Coverage Δ
integration	`48.7482% <90.9090%> (?)`
unit	`72.2674% <72.7272%> (-0.0171%)`	⬇️

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

Components	Coverage Δ
dumpling	`52.6910% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`61.2336% <ø> (+15.4570%)`	⬆️

mjonss

LGTM, only missing some tests for ALTER TABLE t PARTITION BY ... UPDATE INDEXES (...)

mjonss · 2025-01-07T17:39:54Z

tests/integrationtest/t/globalindex/ddl.test

-create table t (a int, b int, unique key (a)) partition by hash(a) partitions 3;
-- error 8264
-alter table t partition by hash(b) partitions 3;
-alter table t partition by hash(b) partitions 3 update indexes (a global);
-- error 8200
-alter table t add index idxErr (b) global;
-alter table t add unique index idxOK (b) global;
-- error 8200
-create index idxErr on t (b) global;
-create unique index idxOK2 on t (b) global;
-alter table t remove partitioning;
-- error 8200
-alter table t add index idxErr (b) global;
-- error 8200
-alter table t add unique index idxErr (b) global;
-- error 8200
-create index idxErr on t (b) global;
-- error 8200
-create unique index idxErr on t (b) global;
-drop table t;



Why remove all of these? The added ones below does not include repartitioning, alter table t partition by hash(b) partitions 3 update indexes (a global); for example.

I think they are duplicate with L58-L79

mjonss · 2025-01-07T17:47:34Z

tests/integrationtest/t/globalindex/ddl.test

+create index idxOK4 on t (b) global;
+create unique index idxOK5 on t (a) global;
+create unique index idxOK6 on t (b) global;
+


Could you please also add tests for repartitioning a table, where one can switch between GLOBAL and NON-GLOBAL non-unique index? Like:

alter table t partition by key(b) partitions 5 UPDATE INDEXES (idxOK LOCAL, idxOK2 GLOBAL, idxOK3 LOCAL);

Ideally so that non-unique indexes will test local->local, local->global, global->local and nglobal->global (both implicitly and explicitly, where possible)

ti-chi-bot · 2025-01-07T17:53:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mjonss, tangenta

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [mjonss,tangenta]
~~pkg/ddl/OWNERS~~ [tangenta]

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 · 2025-01-07T17:54:03Z

[LGTM Timeline notifier]

Timeline:

2025-01-07 07:39:52.674094362 +0000 UTC m=+252935.962926067: ☑️ agreed by tangenta.
2025-01-07 17:54:01.919911062 +0000 UTC m=+289785.208742766: ☑️ agreed by mjonss.

Defined2014 · 2025-01-08T05:58:24Z

/unhold

…long-vector * commit '510d0037b18f258f505abc6cf13a8128563e9359': *: upgrade pd client to make sure tso client initiate successfully (#58752) ttl, test: scale TTL workers during the fault tests (#58750) planner: improve warning messages for unsupported HASH_JOIN hints (#58646) planner: prealloc the slices in the SplitCorColAccessCondFromFilters (#58785) ddl: supports non-unique global index (#58678) util/stmtsummary: add the network traffic related fields (#58101) var: enable `pd_enable_follower_handle_region` as default (#58385) statistics: refactor stats meta handling to use DeltaUpdate for multi-table support (#58657) parser: move 'model' to 'ast' pkg (#58704) statistics: add recover to protect background task (#58739) disttask: cancel subtask context if scheduled away (#58615) *: don't handle live updates of column size (#58596) *: fix a bug for default_authentication_plugin (2) (#58723) dupdetect: gRPC cancel should trigger retry (#58542) *: fix a bug for default_authentication_plugin (#57391) distsql: Fix backoff execution info inaccurate issue (#58707)

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 3, 2025

Defined2014 changed the title ~~ddl: remove limitation and update some test cases~~ ddl: supports non-unique global index Jan 3, 2025

Defined2014 added 3 commits January 3, 2025 14:58

remove limitation and update some test cases

32856e7

update

72c706a

update

f5aa86b

Defined2014 force-pushed the support-non-unique-global-index branch from 91cef4a to f5aa86b Compare January 3, 2025 08:51

ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 3, 2025

Defined2014 added 2 commits January 3, 2025 17:14

update

3f778ba

update

ac18d7d

Defined2014 requested a review from mjonss January 6, 2025 01:49

pingcap deleted a comment from ti-chi-bot bot Jan 6, 2025

Defined2014 requested a review from tangenta January 6, 2025 02:11

tangenta approved these changes Jan 7, 2025

View reviewed changes

ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 7, 2025

mjonss reviewed Jan 7, 2025

View reviewed changes

mjonss added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 7, 2025

mjonss approved these changes Jan 7, 2025

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 7, 2025

add more test cases

0c5529c

ti-chi-bot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 8, 2025

ti-chi-bot bot merged commit d1ed962 into pingcap:master Jan 8, 2025
25 checks passed

Defined2014 deleted the support-non-unique-global-index branch January 8, 2025 06:03

Defined2014 mentioned this pull request Jan 8, 2025

partitioned-table: global index supports non-unique index pingcap/docs-cn#19507

Open

15 tasks

Defined2014 mentioned this pull request Jan 9, 2025

partitioned-table: global index supports non-unique index pingcap/docs#19944

Open

15 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl: supports non-unique global index #58678

ddl: supports non-unique global index #58678

Defined2014 commented Jan 3, 2025 •

edited

Loading

tiprow bot commented Jan 3, 2025

codecov bot commented Jan 3, 2025 •

edited

Loading

mjonss left a comment

mjonss Jan 7, 2025

Defined2014 Jan 8, 2025 •

edited

Loading

mjonss Jan 7, 2025

Defined2014 Jan 8, 2025

ti-chi-bot bot commented Jan 7, 2025

ti-chi-bot bot commented Jan 7, 2025

Defined2014 commented Jan 8, 2025

ddl: supports non-unique global index #58678

ddl: supports non-unique global index #58678

Conversation

Defined2014 commented Jan 3, 2025 • edited Loading

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

tiprow bot commented Jan 3, 2025

codecov bot commented Jan 3, 2025 • edited Loading

Codecov Report

mjonss left a comment

Choose a reason for hiding this comment

mjonss Jan 7, 2025

Choose a reason for hiding this comment

Defined2014 Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

mjonss Jan 7, 2025

Choose a reason for hiding this comment

Defined2014 Jan 8, 2025

Choose a reason for hiding this comment

ti-chi-bot bot commented Jan 7, 2025

ti-chi-bot bot commented Jan 7, 2025

[LGTM Timeline notifier]

Defined2014 commented Jan 8, 2025

Defined2014 commented Jan 3, 2025 •

edited

Loading

codecov bot commented Jan 3, 2025 •

edited

Loading

Defined2014 Jan 8, 2025 •

edited

Loading