Skip to content

Commit

Permalink
infoschema: fix panic for TableByID when table id is negative (#52016)
Browse files Browse the repository at this point in the history
close #52012
  • Loading branch information
lcwangchao authored Mar 27, 2024
1 parent b942197 commit b96f081
Show file tree
Hide file tree
Showing 10 changed files with 79 additions and 5 deletions.
2 changes: 1 addition & 1 deletion br/pkg/restore/db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ func TestGetExistedUserDBs(t *testing.T) {
{Name: model.NewCIStr("d1")},
{
Name: model.NewCIStr("test"),
Tables: []*model.TableInfo{{Name: model.NewCIStr("t1"), State: model.StatePublic}},
Tables: []*model.TableInfo{{ID: 1, Name: model.NewCIStr("t1"), State: model.StatePublic}},
State: model.StatePublic,
},
},
Expand Down
4 changes: 3 additions & 1 deletion pkg/infoschema/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"github.com/pingcap/tidb/pkg/table"
"github.com/pingcap/tidb/pkg/table/tables"
"github.com/pingcap/tidb/pkg/util/domainutil"
"github.com/pingcap/tidb/pkg/util/intest"
)

// Builder builds a new InfoSchema.
Expand Down Expand Up @@ -970,9 +971,10 @@ func NewBuilder(r autoid.Requirement, factory func() (pools.Resource, error), in
}

func tableBucketIdx(tableID int64) int {
intest.Assert(tableID > 0)
return int(tableID % bucketCount)
}

func tableIDIsValid(tableID int64) bool {
return tableID != 0
return tableID > 0
}
18 changes: 18 additions & 0 deletions pkg/infoschema/infoschema.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,17 @@ func MockInfoSchema(tbList []*model.TableInfo) InfoSchema {
tables: make(map[string]table.Table),
}
result.schemaMap["test"] = tableNames
var tableIDs map[int64]struct{}
for _, tb := range tbList {
intest.AssertFunc(func() bool {
if tableIDs == nil {
tableIDs = make(map[int64]struct{})
}
_, ok := tableIDs[tb.ID]
intest.Assert(!ok)
tableIDs[tb.ID] = struct{}{}
return true
})
tb.DBID = dbInfo.ID
tbl := table.MockTableFromMeta(tb)
tableNames.tables[tb.Name.L] = tbl
Expand Down Expand Up @@ -238,6 +248,10 @@ func SchemaByTable(is InfoSchema, tableInfo *model.TableInfo) (val *model.DBInfo
}

func (is *infoSchema) TableByID(id int64) (val table.Table, ok bool) {
if !tableIDIsValid(id) {
return nil, false
}

slice := is.sortedTablesBuckets[tableBucketIdx(id)]
idx := slice.searchTable(id)
if idx == -1 {
Expand Down Expand Up @@ -713,6 +727,10 @@ func (ts *SessionExtendedInfoSchema) FindTableInfoByPartitionID(

// TableByID implements InfoSchema.TableByID
func (ts *SessionExtendedInfoSchema) TableByID(id int64) (table.Table, bool) {
if !tableIDIsValid(id) {
return nil, false
}

if ts.LocalTemporaryTables != nil {
if tbl, ok := ts.LocalTemporaryTables.TableByID(id); ok {
return tbl, true
Expand Down
27 changes: 27 additions & 0 deletions pkg/infoschema/infoschema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,14 @@ func TestBasic(t *testing.T) {
require.False(t, ok)
require.Nil(t, gotTblInfo)

tb, ok = is.TableByID(-12345)
require.False(t, ok)
require.Nil(t, tb)

gotTblInfo, ok = is.TableInfoByID(-12345)
require.False(t, ok)
require.Nil(t, gotTblInfo)

tb, err = is.TableByName(dbName, tbName)
require.NoError(t, err)
require.NotNil(t, tb)
Expand All @@ -187,6 +195,17 @@ func TestBasic(t *testing.T) {
require.Error(t, err)
require.Nil(t, gotTblInfo)

// negative id should always be seen as not exists
tb, ok = is.TableByID(-1)
require.False(t, ok)
require.Nil(t, tb)
schema, ok = is.SchemaByID(-1)
require.False(t, ok)
require.Nil(t, schema)
gotTblInfo, ok = is.TableInfoByID(-1)
require.Nil(t, gotTblInfo)
require.False(t, ok)

tbs := is.SchemaTables(dbName)
require.Len(t, tbs, 1)

Expand Down Expand Up @@ -855,6 +874,14 @@ func TestLocalTemporaryTables(t *testing.T) {
info, ok = is.SchemaByID(tb22.Meta().DBID)
require.False(t, ok)
require.Nil(t, info)

// negative id should always be seen as not exists
tbl, ok = is.TableByID(-1)
require.False(t, ok)
require.Nil(t, tbl)
info, ok = is.SchemaByID(-1)
require.False(t, ok)
require.Nil(t, info)
}

// TestInfoSchemaCreateTableLike tests the table's column ID and index ID for memory database.
Expand Down
4 changes: 4 additions & 0 deletions pkg/infoschema/infoschema_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,10 @@ func (is *infoschemaV2) base() *infoSchema {
}

func (is *infoschemaV2) TableByID(id int64) (val table.Table, ok bool) {
if !tableIDIsValid(id) {
return
}

// Get from the cache.
key := tableCacheKey{id, is.infoSchema.schemaMetaVersion}
tbl, found := is.tableCache.Get(key)
Expand Down
11 changes: 11 additions & 0 deletions pkg/infoschema/infoschema_v2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,17 @@ func TestV2Basic(t *testing.T) {
require.True(t, ok)
require.Same(t, gotTblInfo, getTableInfo.Meta())

// negative id should always be seen as not exists
getTableInfo, ok = is.TableByID(-1)
require.False(t, ok)
require.Nil(t, getTableInfo)
gotTblInfo, ok = is.TableInfoByID(-1)
require.False(t, ok)
require.Nil(t, gotTblInfo)
getDBInfo, ok = is.SchemaByID(-1)
require.False(t, ok)
require.Nil(t, getDBInfo)

gotTblInfo, ok = is.TableInfoByID(1234567)
require.False(t, ok)
require.Nil(t, gotTblInfo)
Expand Down
2 changes: 1 addition & 1 deletion pkg/planner/core/logical_plans_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func createPlannerSuite() (s *plannerSuite) {
MockListPartitionTable(),
MockStateNoneColumnTable(),
}
id := int64(0)
id := int64(1)
for _, tblInfo := range tblInfos {
tblInfo.ID = id
id += 1
Expand Down
8 changes: 8 additions & 0 deletions pkg/planner/core/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ func MockSignedTable() *model.TableInfo {
col5.SetFlag(mysql.NotNullFlag)
col6.SetFlag(mysql.NoDefaultValueFlag)
table := &model.TableInfo{
ID: 1,
Columns: []*model.ColumnInfo{pkColumn, col0, col1, col2, col3, colStr1, colStr2, colStr3, col4, col5, col6, col7},
Indices: indices,
Name: model.NewCIStr("t"),
Expand Down Expand Up @@ -336,6 +337,7 @@ func MockUnsignedTable() *model.TableInfo {
col0.SetFlag(mysql.NotNullFlag)
col1.SetFlag(mysql.UnsignedFlag)
table := &model.TableInfo{
ID: 2,
Columns: []*model.ColumnInfo{pkColumn, col0, col1},
Indices: indices,
Name: model.NewCIStr("t2"),
Expand Down Expand Up @@ -365,6 +367,7 @@ func MockNoPKTable() *model.TableInfo {
col0.SetFlag(mysql.NotNullFlag)
col1.SetFlag(mysql.UnsignedFlag)
table := &model.TableInfo{
ID: 3,
Columns: []*model.ColumnInfo{col0, col1},
Name: model.NewCIStr("t3"),
PKIsHandle: true,
Expand Down Expand Up @@ -395,6 +398,7 @@ func MockView() *model.TableInfo {
}
view := &model.ViewInfo{SelectStmt: selectStmt, Security: model.SecurityDefiner, Definer: &auth.UserIdentity{Username: "root", Hostname: ""}, Cols: []model.CIStr{col0.Name, col1.Name, col2.Name}}
table := &model.TableInfo{
ID: 4,
Name: model.NewCIStr("v"),
Columns: []*model.ColumnInfo{col0, col1, col2},
View: view,
Expand Down Expand Up @@ -462,6 +466,7 @@ func MockRangePartitionTable() *model.TableInfo {
},
}
tableInfo := MockSignedTable()
tableInfo.ID = 5
tableInfo.Name = model.NewCIStr("pt1")
cols := make([]*model.ColumnInfo, 0, len(tableInfo.Columns))
cols = append(cols, tableInfo.Columns...)
Expand Down Expand Up @@ -497,6 +502,7 @@ func MockHashPartitionTable() *model.TableInfo {
},
}
tableInfo := MockSignedTable()
tableInfo.ID = 6
tableInfo.Name = model.NewCIStr("pt2")
cols := make([]*model.ColumnInfo, 0, len(tableInfo.Columns))
cols = append(cols, tableInfo.Columns...)
Expand Down Expand Up @@ -543,6 +549,7 @@ func MockListPartitionTable() *model.TableInfo {
},
}
tableInfo := MockSignedTable()
tableInfo.ID = 7
tableInfo.Name = model.NewCIStr("pt3")
cols := make([]*model.ColumnInfo, 0, len(tableInfo.Columns))
cols = append(cols, tableInfo.Columns...)
Expand Down Expand Up @@ -610,6 +617,7 @@ func MockStateNoneColumnTable() *model.TableInfo {
col0.SetFlag(mysql.NotNullFlag)
col1.SetFlag(mysql.UnsignedFlag)
table := &model.TableInfo{
ID: 8,
Columns: []*model.ColumnInfo{pkColumn, col0, col1},
Indices: indices,
Name: model.NewCIStr("T_StateNoneColumn"),
Expand Down
4 changes: 2 additions & 2 deletions pkg/ttl/ttlworker/job_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,8 @@ func TestReadyForLockHBTimeoutJobTables(t *testing.T) {
tables := m.readyForLockHBTimeoutJobTables(se.Now())
if c.shouldSchedule {
assert.Len(t, tables, 1)
assert.Equal(t, int64(0), tables[0].ID)
assert.Equal(t, int64(0), tables[0].TableInfo.ID)
assert.Equal(t, tbl.ID, tables[0].ID)
assert.Equal(t, tbl.ID, tables[0].TableInfo.ID)
} else {
assert.Len(t, tables, 0)
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/ttl/ttlworker/session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"errors"
"strings"
"sync/atomic"
"testing"
"time"

Expand All @@ -36,8 +37,11 @@ import (
"github.com/stretchr/testify/require"
)

var idAllocator atomic.Int64

func newMockTTLTbl(t *testing.T, name string) *cache.PhysicalTable {
tblInfo := &model.TableInfo{
ID: idAllocator.Add(1),
Name: model.NewCIStr(name),
Columns: []*model.ColumnInfo{
{
Expand Down

0 comments on commit b96f081

Please sign in to comment.