ddl: support create/drop expression index #14117

wjhuang2016 · 2019-12-18T09:34:28Z

What problem does this PR solve?

support create/drop expression index

What is changed and how it works?

For create expression index, we build hidden columns and put it into tableInfo, since it always in the end of column, we don't need to adjust its offsets in wrieOrg.
For drop expression index, we need to remove hidden columns.
Becase we need to remove hidden columns in drop expression index, we can't roll back in state WriteOnly. This will affect Drop Primary Index and Drop Index. We can roll back them in state WriteOnly theoretically, and it will be handled in the future.

Check List

Tests

Unit test
Integration test

Code changes

Has exported function/method change

Side effects

Related changes

Release note

Support create/drop expression index

wjhuang2016 · 2019-12-19T04:18:17Z

/run-all-tests

wjhuang2016 · 2019-12-19T04:22:00Z

/run-all-tests

…crate_index

ddl/db_test.go

ddl/ddl_api.go

util/admin/admin.go

ddl/db_integration_test.go

ddl/ddl_api.go

…x_crate_index

ddl/ddl_api.go

…x_crate_index

sre-bot · 2020-01-05T11:28:37Z

@Deardrops, @zimulala, @AilinKid, PTAL.

zimulala · 2020-01-05T16:06:39Z

ddl/db_integration_test.go

@@ -1896,3 +1898,68 @@ func (s *testIntegrationSuite3) TestParserIssue284(c *C) {
 	tk.MustExec("drop table test.t_parser_issue_284")
 	tk.MustExec("drop table test.t_parser_issue_284_2")
 }
+
+func (s *testIntegrationSuite6) TestAddExpressionIndex(c *C) {


Please add these tests to partition tables. And add a rollback test to test the add index operation, such as testAddIndexRollback.

zimulala · 2020-01-05T16:38:46Z

ddl/db_integration_test.go

+	c.Assert(columns[3].Hidden, IsTrue)
+	c.Assert(columns[4].Hidden, IsTrue)
+
+	tk.MustQuery("select * from t;").Check(testkit.Rows("1 2.1"))


Could we add a test for index data like select b from t use index(idx_multi) where ...?

This pr can't use expression index, should we add test using expression index explicitly?

The test for index data will be added in following PR.

ddl/index.go

zimulala · 2020-01-05T17:00:59Z

executor/show_test.go

@@ -609,6 +609,19 @@ func (s *testSuite5) TestShowCreateTable(c *C) {
 			"  `a` varchar(123) DEFAULT NULL\n"+
 			") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin",
 	))
+


Do we need to handle the SQL of show keys from t;? It seems that we need to add a field of Expression.

Yeah, I will support this in following PR.

zimulala · 2020-01-05T17:24:24Z

ddl/column.go

@@ -77,6 +77,9 @@ func adjustColumnInfoInDropColumn(tblInfo *model.TableInfo, offset int) {
 		oldCols[i].Offset = i - 1
 	}
 	oldCols[offset].Offset = len(oldCols) - 1
+	// For expression index, we drop hidden columns and index simultaneously.
+	// So we need to change the offset of expression index.
+	offsetChanged[offset] = len(oldCols) - 1


Could we remove this logic? It seems only the current expression index will use this hidden column. So we needn't update this column's offset when dropping this expression index.

If remove this logic, the column offset of the expression index will be wrong.

I got it. At this time, the expression index only modifies the state, and it has not been actually deleted.

ddl/ddl_api.go

ddl/index.go

ddl/rollingback.go

executor/show_test.go

util/admin/admin.go

AilinKid · 2020-01-06T08:59:25Z

LGTM

zimulala

LGTM

crazycs520

LGTM

sre-bot · 2020-01-07T02:26:41Z

/run-all-tests

wjhuang2016 added 3 commits December 18, 2019 17:23

save

d85a46a

merge master

c89d7c1

save

8ee0ae7

wjhuang2016 requested a review from a team as a code owner December 18, 2019 09:34

ghost requested review from eurekaka and lzmhhh123 and removed request for a team December 18, 2019 09:34

wjhuang2016 added status/WIP type/new-feature labels Dec 18, 2019

eurekaka removed their request for review December 18, 2019 09:35

lzmhhh123 removed their request for review December 18, 2019 09:39

wjhuang2016 added 3 commits December 18, 2019 18:01

save

a3b5ab1

tweak

b999122

tweak

a5bb03c

wjhuang2016 added 2 commits December 19, 2019 14:30

clean code

133428e

Merge remote-tracking branch 'upstream/master' into expression_index_…

5c5e4f9

…crate_index

wjhuang2016 force-pushed the expression_index_crate_index branch from 65301fc to 5c5e4f9 Compare December 19, 2019 08:45

wjhuang2016 added 3 commits December 20, 2019 14:43

save

634e2a9

Merge remote-tracking branch 'upstream/master' into expression_index_…

ca4a262

…crate_index

fix test

149d728

wjhuang2016 added status/PTAL and removed status/WIP labels Dec 20, 2019

Merge branch 'master' into expression_index_crate_index

018620e

Deardrops reviewed Dec 25, 2019

View reviewed changes

ddl/db_test.go Show resolved Hide resolved

ddl/ddl_api.go Show resolved Hide resolved

ddl/ddl_api.go Outdated Show resolved Hide resolved

ddl/ddl_api.go Outdated Show resolved Hide resolved

Merge branch 'master' into expression_index_crate_index

3aade76

Deardrops reviewed Dec 25, 2019

View reviewed changes

util/admin/admin.go Outdated Show resolved Hide resolved

zimulala reviewed Dec 25, 2019

View reviewed changes

wjhuang2016 force-pushed the expression_index_crate_index branch from 03babd7 to 1abd378 Compare January 2, 2020 05:09

wjhuang2016 added 3 commits January 2, 2020 13:10

lint

40b9b07

tweak

fcd8385

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

40a5ea1

…x_crate_index

zimulala reviewed Jan 3, 2020

View reviewed changes

ddl/ddl_api.go Show resolved Hide resolved

wjhuang2016 added 2 commits January 3, 2020 15:58

fix col.dependence is nil

1a40266

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

0fbfd50

…x_crate_index

wjhuang2016 force-pushed the expression_index_crate_index branch from 4117477 to 0fbfd50 Compare January 3, 2020 07:59

zimulala reviewed Jan 5, 2020

View reviewed changes

zimulala added the status/LGT1 Indicates that a PR has LGTM 1. label Jan 6, 2020

crazycs520 reviewed Jan 6, 2020

View reviewed changes

ddl/ddl_api.go Show resolved Hide resolved

AilinKid reviewed Jan 6, 2020

View reviewed changes

ddl/index.go Outdated Show resolved Hide resolved

ddl/rollingback.go Outdated Show resolved Hide resolved

executor/show_test.go Show resolved Hide resolved

util/admin/admin.go Outdated Show resolved Hide resolved

util/admin/admin.go Outdated Show resolved Hide resolved

wjhuang2016 added 2 commits January 6, 2020 12:41

address comment

edace5d

address comment

4938b40

wjhuang2016 force-pushed the expression_index_crate_index branch from 56178cf to 4938b40 Compare January 6, 2020 04:55

address comment

8e1acba

AilinKid added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jan 6, 2020

zimulala reviewed Jan 6, 2020

View reviewed changes

crazycs520 approved these changes Jan 7, 2020

View reviewed changes

zimulala added status/can-merge Indicates a PR has been approved by a committer. status/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Jan 7, 2020

Merge branch 'master' into expression_index_crate_index

8c19669

sre-bot merged commit f036eec into pingcap:master Jan 7, 2020

you06 added the sig/sql-infra SIG: SQL Infra label Mar 4, 2020

wjhuang2016 deleted the expression_index_crate_index branch November 17, 2022 11:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl: support create/drop expression index #14117

ddl: support create/drop expression index #14117

wjhuang2016 commented Dec 18, 2019 •

edited

Loading

wjhuang2016 commented Dec 19, 2019

wjhuang2016 commented Dec 19, 2019

sre-bot commented Jan 5, 2020

zimulala Jan 5, 2020

wjhuang2016 Jan 6, 2020

zimulala Jan 5, 2020

wjhuang2016 Jan 6, 2020

wjhuang2016 Jan 6, 2020

zimulala Jan 5, 2020

wjhuang2016 Jan 6, 2020

zimulala Jan 5, 2020

wjhuang2016 Jan 6, 2020

zimulala Jan 6, 2020

AilinKid commented Jan 6, 2020

zimulala left a comment

crazycs520 left a comment

sre-bot commented Jan 7, 2020

ddl: support create/drop expression index #14117

ddl: support create/drop expression index #14117

Conversation

wjhuang2016 commented Dec 18, 2019 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

wjhuang2016 commented Dec 19, 2019

wjhuang2016 commented Dec 19, 2019

sre-bot commented Jan 5, 2020

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

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

AilinKid commented Jan 6, 2020

zimulala left a comment

Choose a reason for hiding this comment

crazycs520 left a comment

Choose a reason for hiding this comment

sre-bot commented Jan 7, 2020

wjhuang2016 commented Dec 18, 2019 •

edited

Loading