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.

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, transaction: DDL on temporary tables won't affect transactions #24534

Merged
merged 11 commits into from
May 17, 2021
5 changes: 3 additions & 2 deletions domain/schema_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,9 @@ func (s *schemaValidator) Check(txnTS uint64, schemaVer int64, relatedPhysicalTa

// Schema changed, result decided by whether related tables change.
if schemaVer < s.latestSchemaVer {
// The DDL relatedPhysicalTableIDs is empty.
if len(relatedPhysicalTableIDs) == 0 {
// When a transaction executes a DDL, relatedPhysicalTableIDs is nil.
// When a transaction only contains DML on temporary tables, relatedPhysicalTableIDs is [].
if relatedPhysicalTableIDs == nil {
tiancaiamao marked this conversation as resolved.
Show resolved Hide resolved
logutil.BgLogger().Info("the related physical table ID is empty", zap.Int64("schemaVer", schemaVer),
zap.Int64("latestSchemaVer", s.latestSchemaVer))
return nil, ResultFail
Expand Down
6 changes: 6 additions & 0 deletions session/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -505,8 +505,14 @@ func (s *session) doCommit(ctx context.Context) error {

// Get the related table or partition IDs.
relatedPhysicalTables := s.GetSessionVars().TxnCtx.TableDeltaMap
// Get accessed global temporary tables in the transaction.
temporaryTables := s.GetSessionVars().TxnCtx.GlobalTemporaryTables
physicalTableIDs := make([]int64, 0, len(relatedPhysicalTables))
for id := range relatedPhysicalTables {
// Schema change on global temporary tables doesn't affect transactions.
if _, ok := temporaryTables[id]; ok {
continue
}
physicalTableIDs = append(physicalTableIDs, id)
}
// Set this option for 2 phase commit to validate schema lease.
Expand Down
39 changes: 39 additions & 0 deletions session/session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2131,6 +2131,45 @@ func (s *testSchemaSerialSuite) TestSchemaCheckerSQL(c *C) {
c.Assert(err, NotNil)
}

func (s *testSchemaSerialSuite) TestSchemaCheckerTempTable(c *C) {
tk := testkit.NewTestKitWithInit(c, s.store)
tk1 := testkit.NewTestKitWithInit(c, s.store)

// create table
tk.MustExec(`drop table if exists normal_table`)
tk.MustExec(`create table normal_table (id int, c int);`)
defer tk.MustExec(`drop table if exists normal_table`)
tk.MustExec(`drop table if exists temp_table`)
tk.MustExec(`create global temporary table temp_table (id int, c int) on commit delete rows;`)
defer tk.MustExec(`drop table if exists temp_table`)

// The schema version is out of date in the first transaction, and the SQL can't be retried.
atomic.StoreUint32(&session.SchemaChangedWithoutRetry, 1)
defer func() {
atomic.StoreUint32(&session.SchemaChangedWithoutRetry, 0)
}()

// It's fine to change the schema of temporary tables.
tk.MustExec(`begin;`)
tk1.MustExec(`alter table temp_table modify column c bigint;`)
tk.MustExec(`insert into temp_table values(3, 3);`)
tk.MustExec(`commit;`)

// Truncate will modify table ID.
tk.MustExec(`begin;`)
tk1.MustExec(`truncate table temp_table;`)
tk.MustExec(`insert into temp_table values(3, 3);`)
tk.MustExec(`commit;`)

// It reports error when also changing the schema of a normal table.
tk.MustExec(`begin;`)
tk1.MustExec(`alter table normal_table modify column c bigint;`)
tk.MustExec(`insert into temp_table values(3, 3);`)
tk.MustExec(`insert into normal_table values(3, 3);`)
_, err := tk.Exec(`commit;`)
c.Assert(terror.ErrorEqual(err, domain.ErrInfoSchemaChanged), IsTrue, Commentf("err %v", err))
}

func (s *testSchemaSuite) TestPrepareStmtCommitWhenSchemaChanged(c *C) {
tk := testkit.NewTestKitWithInit(c, s.store)
tk1 := testkit.NewTestKitWithInit(c, s.store)
Expand Down