ddl: support add global index operation on partition table #18402

ldeng-ustc · 2020-07-07T06:31:02Z

What problem does this PR solve?

Problem Summary:

Support adding global indices. It is a part of #18032 .

What is changed and how it works?

Proposal: #18982

What's Changed:

New feature: Support add global index.

When global index is enabled, constraint in partition table index will be removed. If config.EnableGlobalIndex is set, user can add global index by adding an index which value columns do not include all partition columns.

such as:

create table t (a int, b int) partition by range(b) (partition p0 values less than (10));
alter table t add unique idx (a);

Before this PR:

ERROR 1503 (HY000): A UNIQUE INDEX must include all columns in the table's partitioning function

After (set EnableGlobalIndex):

Query OK, 0 rows affected (0.05 sec)

How it Works:

model.IndexInfo was modified to add Global field in paser PR #917.

Decide whether index is global before ddl job is enqueued.

In global index, tableID (i.e. TableInfo.ID) instead of physicalTableID was used to create index prefix, meanwhile, partitionID is add into value layout.

tablecodec.GenIndexValueNew support encode global index value.

Finally, add a testcase to test whether data is correctly writed into kv.

Index Value Layout

Now, we have 6 index value layouts, and use 3 different function to decode it. This not only makes it hard to add new features for index (we should add at least two layouts and one function), but also makes the code and comments less readable and confusing (index.Create has an extreme long comment, and is still getting longer).

In fact, we can just divide the encoding into two layout: new and old. Old encoding value has no specific part, and length always <= 9. On the other hand, new encoding value has >= 10 length, and include some segments for specific features (e.g. restoreData for new collation, common handle, and also, partitionID for global index).

So, I refactor tablecodec.GenIndexValue and tablecodec.DecodeIndexKV for extensibilty, and rewrite comment of index.Create in a more readable way. After that, we can easily add new features to index value: simply add a segment to value.

Here is the new description of index value layout. Note that I just change the decription, so this encoding is exactly same as origin (expect additional global index segment). In other words, there is no compatibility issue.

// Create creates a new entry in the kvIndex data.
// If the index is unique and there is an existing entry with the same key,
// Create will return the existing entry's handle as the first return value, ErrKeyExists as the second return value.
// Value layout:
//		+--New Encoding (with restore data, or common handle, or index is global)
//		|
//		|  Layout: TailLen | Options      | Padding      | [IntHandle] | [UntouchedFlag]
//		|  Length:   1     | len(options) | len(padding) |    8        |     1
//		|
//		|  TailLen:       len(padding) + len(IntHandle) + len(UntouchedFlag)
//		|  Options:       Encode some value for new features, such as common handle, new collations or global index.
//		|                 See below for more information.
//		|  Padding:       Ensure length of value always >= 10. (or >= 11 if UntouchedFlag exists.)
//		|  IntHandle:     Only exists when table use int handles and index is unique.
//		|  UntouchedFlag: Only exists when index is untouched.
//		|
//		|  Layout of Options:
//		|
//		|     Segment:             Common Handle                 |     Global Index      | New Collation
//		|     Layout:  CHandle Flag | CHandle Len | CHandle      | PidFlag | PartitionID | restoreData
//		|     Length:     1         | 2           | len(CHandle) |    1    |    8        | len(restoreData)
//		|
//		|     Common Handle Segment: Exists when unique index used common handles.
//		|     Global Index Segment:  Exists when index is global.
//		|     New Collation Segment: Exists when new collation is used and index contains non-binary string.
//		|
//		+--Old Encoding (without restore data, integer handle, local)
//
//		   Layout: [Handle] | [UntouchedFlag]
//		   Length:   8      |     1
//
//		   Handle:        Only exists in unique index.
//		   UntouchedFlag: Only exists when index is untouched.
//
//		   If neither Handle nor UntouchedFlag exists, value will be one single byte '0' (i.e. []byte{'0'}).
//		   Length of value <= 9, use to distinguish from the new encoding.
//

Related changes

PR to update pingcap/parser: #917

Check List

Tests

Unit test

Side effects

None.

Release note

No release note.

ddl/index.go

ddl/db_test.go

ddl/ddl_api.go

ddl/partition.go

table/tables/index.go

tiancaiamao · 2020-07-09T08:19:43Z

tablecodec/tablecodec.go

@@ -971,8 +971,8 @@ func GenIndexKey(sc *stmtctx.StatementContext, tblInfo *model.TableInfo, idxInfo
 }

 // GenIndexValue creates encoded index value and returns the result
-func GenIndexValue(sc *stmtctx.StatementContext, tblInfo *model.TableInfo, idxInfo *model.IndexInfo,
-	containNonBinaryString bool, distinct bool, untouched bool, indexedValues []types.Datum, h kv.Handle) ([]byte, error) {
+func GenIndexValue(sc *stmtctx.StatementContext, tblInfo *model.TableInfo, idxInfo *model.IndexInfo, containNonBinaryString bool,


If there are multiple reference of this function in other packages, it's better to introduce a new function such as GenGlobalIndexValue instead of changing the API

This API does not change, I just split a long single line into two.

You add a partitionID int64, right?

tiancaiamao · 2020-07-09T16:07:38Z

ddl/db_test.go

+	c.Assert(rowValueDatums[cols[1].ID], DeepEquals, types.NewDatum(1))
+
+	// check row 2
+	key, _, err = tablecodec.GenIndexKey(sc, tblInfo, indexInfo, tblInfo.ID, []types.Datum{types.NewDatum(2)}, kv.IntHandle(1), nil)


check row1 and row2 code are basically the same, you could define a local function to reduce repeat code.

tiancaiamao · 2020-07-09T16:21:23Z

tablecodec/tablecodec.go

@@ -971,8 +971,8 @@ func GenIndexKey(sc *stmtctx.StatementContext, tblInfo *model.TableInfo, idxInfo
 }

 // GenIndexValue creates encoded index value and returns the result
-func GenIndexValue(sc *stmtctx.StatementContext, tblInfo *model.TableInfo, idxInfo *model.IndexInfo,
-	containNonBinaryString bool, distinct bool, untouched bool, indexedValues []types.Datum, h kv.Handle) ([]byte, error) {
+func GenIndexValue(sc *stmtctx.StatementContext, tblInfo *model.TableInfo, idxInfo *model.IndexInfo, containNonBinaryString bool,


You add a partitionID int64, right?

tiancaiamao · 2020-07-09T16:30:12Z

I'm not sure the quite sure the influence of alter table add primary key on the global index encoding.
Rest LGTM

tiancaiamao · 2020-07-09T16:30:35Z

PTAL @wjhuang2016 @crazycs520

coocood · 2020-07-10T04:19:27Z

table/tables/index.go

+//		|  |  |
+//		|  |  |     The length >= 19 always because of padding.
+//		|  |  |
+//		... (Truncated because of length.)


Why truncated?

New layout of local index is just add 8 bytes partitionID at the end, rest part is same as local index . I think 2 examples is enough. If write whole layout here, it seems too long.

wjhuang2016 · 2020-07-10T04:30:30Z

ddl/db_integration_test.go

@@ -1169,7 +1169,7 @@ func (s *testIntegrationSuite5) TestBackwardCompatibility(c *C) {
 		TableID:    tbl.Meta().ID,
 		Type:       model.ActionAddIndex,
 		BinlogInfo: &model.HistoryInfo{},
-		Args:       []interface{}{unique, indexName, indexPartSpecifications, indexOption},
+		Args:       []interface{}{unique, false, indexName, indexPartSpecifications, indexOption},


This may cause a problem when rolling upgrades. A new TiDB receives old args sent by old TiDB.

Is there a better way to pass one more arg to worker? I have no idea.

Put it in the last arg

Add new field in model.IndexInfo is backward compatible, i.e. a newer TiDB can handle older TiDB.
When the new version find the Global field is missing (by old TiDB), it set the value to false.

A old TiDB receive newer TiDB may be something wrong, because Global may be true and the old server do not know how to handle it, so we must gurantee there is no DDL during rolling update.

wjhuang2016

Please fire a proposal first. We need to understand and check the whole design before reviewing it.

coocood · 2020-07-10T04:48:44Z

Appending a new element to the end of the value is not extensible.
We may need to add more elements in the future.
The better way is to add a partition ID flag, then a partition id, the tail data is unchanged.

ldeng-ustc · 2020-08-04T13:21:02Z

I write the proposal in a new PR.
PTAL @tiancaiamao @wjhuang2016 @coocood

lysu · 2020-08-17T08:06:14Z

LGTM

lysu · 2020-08-17T08:14:55Z

maybe we want to support create global in create table stmt in following PR~?

for example:

 create table test_t1 (a int, b int, unique index p_a (a)) partition by hash (b) partitions 3;

lysu · 2020-08-17T08:18:07Z

ddl/ddl_api.go

 	indexColumns, err := buildIndexColumns(append(tblInfo.Columns, hiddenCols...), indexPartSpecifications)
 	if err != nil {
 		return errors.Trace(err)
 	}
+
+	global := false
 	if unique && tblInfo.GetPartitionInfo() != nil {


tiny requirement question:

do we need support non-unique global index?

cc: @tiancaiamao @zz-jason @coocood

maybe it's useful to support "using a non-unique index without include partition columns", maybe there is a real-world example:

create table order ( oid_id int, user_id int, seller_id int, primary key(oid, user_id), index uid (user_id), ) partition by hash(user_id) partition 1000

this table will conventional in the "user end side" to let the order-system query by user_id.

but the seller also want query the orders belongs itself in seller system and delivery order, and query by seller_id and it didn't know partition_id(also seller_id isn't unique in order table), and maybe we need a global non-unique index --- index sid (seller_id) to help seller request can be prunning?

I agree. In some case global non-unique index is useful. Maybe we should support keywords Local and Global just like Oracle. @zhaox1n had submitted a PR about it, but it is closed now.

Not in this PR.
Support global unique index is complex enough, we should do it step by step.

tiancaiamao · 2020-08-18T12:52:57Z

LGTM

PTAL @coocood @wjhuang2016

tiancaiamao · 2020-08-21T05:48:08Z

/merge

ti-srebot · 2020-08-21T05:48:10Z

@tiancaiamao Oops! This PR requires at least 2 LGTMs to merge. The current number of LGTM is 1.

tiancaiamao · 2020-08-21T05:49:23Z

LGTM

tiancaiamao · 2020-08-21T05:49:40Z

/merge

ti-srebot · 2020-08-21T05:51:57Z

/run-all-tests

ti-srebot · 2020-08-21T05:57:25Z

@ldeng-ustc merge failed.

tiancaiamao · 2020-08-21T06:11:40Z

/run-unit-test

pingcap#18402

ldeng-ustc added 2 commits July 7, 2020 12:41

ddl: support add global index

47f82e1

Merge branch 'master' of github.com:pingcap/tidb

55b09e2

ti-srebot added the contribution This PR is from a community contributor. label Jul 7, 2020

github-actions bot added the sig/sql-infra SIG: SQL Infra label Jul 7, 2020

tiancaiamao reviewed Jul 7, 2020

View reviewed changes

ddl/index.go Outdated Show resolved Hide resolved

ldeng-ustc mentioned this pull request Jul 7, 2020

model: add Global field to IndexInfo pingcap/parser#917

Merged

ldeng-ustc added 2 commits July 9, 2020 15:14

ddl: add config value 'EnableGlobalIndex'

58f90b7

Merge branch 'master' of github.com:pingcap/tidb into global_index

5d47ec6

github-actions bot added the component/config label Jul 9, 2020

tiancaiamao changed the title ~~ddl: support add global index~~ ddl: support add global index operation on partition table Jul 9, 2020

fix

b15b1f2

tiancaiamao reviewed Jul 9, 2020

View reviewed changes

ldeng-ustc added 3 commits July 9, 2020 21:54

tests: update testcase for creating global index

2e04b6c

go.mod: updade parser to support IndexInfo.Global

e5284c6

go.mod: updade parser to support IndexInfo.Global

2faf246

tiancaiamao reviewed Jul 9, 2020

View reviewed changes

zimulala added the require-LGT3 Indicates that the PR requires three LGTM. label Jul 10, 2020

coocood reviewed Jul 10, 2020

View reviewed changes

wjhuang2016 reviewed Jul 10, 2020

View reviewed changes

fix

505a620

tiancaiamao removed the require-LGT3 Indicates that the PR requires three LGTM. label Jul 10, 2020

ldeng-ustc added 5 commits July 27, 2020 16:44

Merge branch 'master' of github.com:pingcap/tidb into global_index

19b0a97

tablecodec: refactor GenIndexValue for extensibility

2febc3a

tablecodec: add decodeIndexKvGeneral for extensible index decoding

9ef4eab

tablecodec: use decodeIndexKvGeneral in DecodeIndexKV

c933af0

Merge branch 'master' of github.com:pingcap/tidb into global_index

5616d86

AndreMouche removed the component/DDL label Aug 11, 2020

ldeng-ustc mentioned this pull request Aug 16, 2020

DDL: support drop partition on the partitioned table with global indexes #19222

Merged

ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Aug 17, 2020

lysu reviewed Aug 17, 2020

View reviewed changes

Merge remote-tracking branch 'pingcap/master' into global_index

3dab738

tiancaiamao added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Aug 18, 2020

ldeng-ustc added 4 commits August 18, 2020 21:50

fix comment

cf92788

fix make check fail

3d75458

Merge remote-tracking branch 'pingcap/master' into global_index

4286a90

Merge branch 'master' of github.com:pingcap/tidb into global_index

516bbf0

ti-srebot approved these changes Aug 21, 2020

View reviewed changes

ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Aug 21, 2020

Merge branch 'master' into global_index

2862e06

tiancaiamao mentioned this pull request Aug 21, 2020

Get TiFlash region cache data race #19353

Closed

tiancaiamao merged commit eb40424 into pingcap:master Aug 21, 2020

tiancaiamao added a commit to tiancaiamao/tidb that referenced this pull request Oct 13, 2020

ddl: support add global index operation on partition table

3f7f95d

pingcap#18402

tiancaiamao mentioned this pull request Oct 13, 2020

ddl: support add global index operation on partition table bb7133/tidb#25

Merged

mjonss mentioned this pull request Feb 14, 2022

Support global index for partitioned tables #18032

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl: support add global index operation on partition table #18402

ddl: support add global index operation on partition table #18402

ldeng-ustc commented Jul 7, 2020 •

edited

Loading

tiancaiamao Jul 9, 2020

ldeng-ustc Jul 9, 2020

tiancaiamao Jul 9, 2020

tiancaiamao Jul 9, 2020

tiancaiamao Jul 9, 2020

tiancaiamao commented Jul 9, 2020

tiancaiamao commented Jul 9, 2020

coocood Jul 10, 2020

ldeng-ustc Jul 10, 2020

wjhuang2016 Jul 10, 2020

ldeng-ustc Jul 10, 2020 •

edited

Loading

wjhuang2016 Jul 10, 2020

tiancaiamao Jul 10, 2020

wjhuang2016 left a comment

coocood commented Jul 10, 2020

ldeng-ustc commented Aug 4, 2020

lysu commented Aug 17, 2020

lysu commented Aug 17, 2020

lysu Aug 17, 2020

lysu Aug 17, 2020 •

edited

Loading

ldeng-ustc Aug 17, 2020

tiancaiamao Aug 18, 2020

tiancaiamao commented Aug 18, 2020

tiancaiamao commented Aug 21, 2020

ti-srebot commented Aug 21, 2020

tiancaiamao commented Aug 21, 2020

tiancaiamao commented Aug 21, 2020

ti-srebot commented Aug 21, 2020

ti-srebot commented Aug 21, 2020

tiancaiamao commented Aug 21, 2020

ddl: support add global index operation on partition table #18402

ddl: support add global index operation on partition table #18402

Conversation

ldeng-ustc commented Jul 7, 2020 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Index Value Layout

Related changes

Check List

Release note

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tiancaiamao commented Jul 9, 2020

tiancaiamao commented Jul 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ldeng-ustc Jul 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wjhuang2016 left a comment

Choose a reason for hiding this comment

coocood commented Jul 10, 2020

ldeng-ustc commented Aug 4, 2020

lysu commented Aug 17, 2020

lysu commented Aug 17, 2020

Choose a reason for hiding this comment

lysu Aug 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tiancaiamao commented Aug 18, 2020

tiancaiamao commented Aug 21, 2020

ti-srebot commented Aug 21, 2020

tiancaiamao commented Aug 21, 2020

tiancaiamao commented Aug 21, 2020

ti-srebot commented Aug 21, 2020

ti-srebot commented Aug 21, 2020

tiancaiamao commented Aug 21, 2020

ldeng-ustc commented Jul 7, 2020 •

edited

Loading

ldeng-ustc Jul 10, 2020 •

edited

Loading

lysu Aug 17, 2020 •

edited

Loading