Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ddl: Do not consider the clustered index when checking the length of the secondary index #29660

Merged
merged 7 commits into from
Dec 1, 2021
Merged

ddl: Do not consider the clustered index when checking the length of the secondary index #29660

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bfc9682
ddl: Do not consider the clustered index when checking the length of …
TonsnakeLin Nov 10, 2021
e1c3410
Merge branch 'master' of https://github.com/TonsnakeLin/tidb
TonsnakeLin Nov 10, 2021
662871c
Merge branch 'master' into master
TonsnakeLin Nov 10, 2021
dce8ef4
Merge branch 'master' into master
TonsnakeLin Nov 10, 2021
68a32a0
ddl: Do not consider the clustered index when checking the length of the
TonsnakeLin Nov 22, 2021
4918824
Merge branch 'master' into master
ti-chi-bot Dec 1, 2021
4d6c1b0
Merge branch 'master' into master
ti-chi-bot Dec 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions ddl/db_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1537,7 +1537,7 @@ func (s *testSerialDBSuite1) TestCreateSecondaryIndexInCluster(c *C) {
tk.MustExec("use test")

// test create table with non-unique key
tk.MustGetErrCode(`
tk.MustExec(`
CREATE TABLE t (
c01 varchar(255) NOT NULL,
c02 varchar(255) NOT NULL,
Expand All @@ -1547,7 +1547,8 @@ CREATE TABLE t (
c06 varchar(255) DEFAULT NULL,
PRIMARY KEY (c01,c02,c03) clustered,
KEY c04 (c04)
)`, errno.ErrTooLongKey)
)`)
tk.MustExec("drop table t")

// test create long clustered primary key.
tk.MustGetErrCode(`
Expand Down Expand Up @@ -1587,7 +1588,7 @@ CREATE TABLE t (
PRIMARY KEY (c01,c02) clustered
)`)
tk.MustExec("create index idx1 on t(c03)")
tk.MustGetErrCode("create index idx2 on t(c03, c04)", errno.ErrTooLongKey)
tk.MustExec("create index idx2 on t(c03, c04)")
tk.MustExec("create unique index uk2 on t(c03, c04)")
tk.MustExec("drop table t")

Expand All @@ -1606,9 +1607,9 @@ CREATE TABLE t (
)`)
tk.MustExec("alter table t change c03 c10 varchar(256) default null")
tk.MustGetErrCode("alter table t change c10 c100 varchar(1024) default null", errno.ErrTooLongKey)
tk.MustGetErrCode("alter table t modify c10 varchar(600) default null", errno.ErrTooLongKey)
tk.MustExec("alter table t modify c10 varchar(600) default null")
tk.MustExec("alter table t modify c06 varchar(600) default null")
tk.MustGetErrCode("alter table t modify c01 varchar(510)", errno.ErrTooLongKey)
tk.MustExec("alter table t modify c01 varchar(510)")
tk.MustExec("create table t2 like t")
}

Expand Down
75 changes: 12 additions & 63 deletions ddl/ddl_api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1631,27 +1631,7 @@ func buildTableInfo(
idxInfo.ID = allocateIndexID(tbInfo)
tbInfo.Indices = append(tbInfo.Indices, idxInfo)
}
if tbInfo.IsCommonHandle {
// Ensure tblInfo's each non-unique secondary-index's len + primary-key's len <= MaxIndexLength for clustered index table.
var pkLen, idxLen int
pkLen, err = indexColumnsLen(tbInfo.Columns, tables.FindPrimaryIndex(tbInfo).Columns)
if err != nil {
return
}
for _, idx := range tbInfo.Indices {
if idx.Unique {
// Only need check for non-unique secondary-index.
continue
}
idxLen, err = indexColumnsLen(tbInfo.Columns, idx.Columns)
if err != nil {
return
}
if pkLen+idxLen > config.GetGlobalConfig().MaxIndexLength {
return nil, errTooLongKey.GenWithStackByArgs(config.GetGlobalConfig().MaxIndexLength)
}
}
}

return
}

Expand Down Expand Up @@ -4272,9 +4252,6 @@ func (d *ddl) getModifiableColumnJob(ctx context.Context, sctx sessionctx.Contex
// Index has a max-prefix-length constraint. eg: a varchar(100), index idx(a), modifying column a to a varchar(4000)
// will cause index idx to break the max-prefix-length constraint.
//
// For clustered index:
// Change column in pk need recheck all non-unique index, new pk len + index len < maxIndexLength.
// Change column in secondary only need related index, pk len + new index len < maxIndexLength.
func checkColumnWithIndexConstraint(tbInfo *model.TableInfo, originalCol, newCol *model.ColumnInfo) error {
columns := make([]*model.ColumnInfo, 0, len(tbInfo.Columns))
columns = append(columns, tbInfo.Columns...)
Expand All @@ -4289,40 +4266,31 @@ func checkColumnWithIndexConstraint(tbInfo *model.TableInfo, originalCol, newCol
}

pkIndex := tables.FindPrimaryIndex(tbInfo)
var clusteredPkLen int
if tbInfo.IsCommonHandle {
var err error
clusteredPkLen, err = indexColumnsLen(columns, pkIndex.Columns)
if err != nil {
return err
}
}

checkOneIndex := func(indexInfo *model.IndexInfo, pkLenAppendToKey int, skipCheckIfNotModify bool) (modified bool, err error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, it seems the return value modified is not used. Can it be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes， I think so.

checkOneIndex := func(indexInfo *model.IndexInfo) (err error) {
var modified bool
for _, col := range indexInfo.Columns {
if col.Name.L == originalCol.Name.L {
modified = true
break
}
}
if skipCheckIfNotModify && !modified {
if !modified {
return
}
err = checkIndexInModifiableColumns(columns, indexInfo.Columns)
if err != nil {
return
}
err = checkIndexPrefixLength(columns, indexInfo.Columns, pkLenAppendToKey)
err = checkIndexPrefixLength(columns, indexInfo.Columns)
return
}

// Check primary key first and get "does primary key's column has be modified?" info.
var (
pkModified bool
err error
)
// Check primary key first.
var err error

if pkIndex != nil {
pkModified, err = checkOneIndex(pkIndex, 0, true)
err = checkOneIndex(pkIndex)
if err != nil {
return err
}
Expand All @@ -4333,12 +4301,9 @@ func checkColumnWithIndexConstraint(tbInfo *model.TableInfo, originalCol, newCol
if indexInfo.Primary {
continue
}
var pkLenAppendToKey int
if !indexInfo.Unique {
pkLenAppendToKey = clusteredPkLen
}

_, err = checkOneIndex(indexInfo, pkLenAppendToKey, !tbInfo.IsCommonHandle || !pkModified)
// the second param should always be set to true, check index length only if it was modified
// checkOneIndex needs one param only.
err = checkOneIndex(indexInfo)
if err != nil {
return err
}
Expand Down Expand Up @@ -5481,22 +5446,6 @@ func (d *ddl) CreateIndex(ctx sessionctx.Context, ti ast.Ident, keyType ast.Inde
return errors.Trace(err)
}

if !unique && tblInfo.IsCommonHandle {
// Ensure new created non-unique secondary-index's len + primary-key's len <= MaxIndexLength in clustered index table.
var pkLen, idxLen int
pkLen, err = indexColumnsLen(tblInfo.Columns, tables.FindPrimaryIndex(tblInfo).Columns)
if err != nil {
return err
}
idxLen, err = indexColumnsLen(finalColumns, indexColumns)
if err != nil {
return err
}
if pkLen+idxLen > config.GetGlobalConfig().MaxIndexLength {
return errTooLongKey.GenWithStackByArgs(config.GetGlobalConfig().MaxIndexLength)
}
}

global := false
if unique && tblInfo.GetPartitionInfo() != nil {
ck, err := checkPartitionKeysConstraint(tblInfo.GetPartitionInfo(), indexColumns, tblInfo)
Expand Down
4 changes: 2 additions & 2 deletions ddl/index.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,12 @@ func checkPKOnGeneratedColumn(tblInfo *model.TableInfo, indexPartSpecifications
return lastCol, nil
}

func checkIndexPrefixLength(columns []*model.ColumnInfo, idxColumns []*model.IndexColumn, pkLenAppendToKey int) error {
func checkIndexPrefixLength(columns []*model.ColumnInfo, idxColumns []*model.IndexColumn) error {
idxLen, err := indexColumnsLen(columns, idxColumns)
if err != nil {
return err
}
if idxLen+pkLenAppendToKey > config.GetGlobalConfig().MaxIndexLength {
if idxLen > config.GetGlobalConfig().MaxIndexLength {
return errTooLongKey.GenWithStackByArgs(config.GetGlobalConfig().MaxIndexLength)
}
return nil
Expand Down