Skip to content

Commit

Permalink
address comment and fix typo
Browse files Browse the repository at this point in the history
  • Loading branch information
bb7133 committed May 15, 2019
1 parent 452d2ee commit 62cb0a1
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 16 deletions.
4 changes: 2 additions & 2 deletions ddl/db_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1645,7 +1645,7 @@ func (s *testIntegrationSuite11) TestChangingDBCharset(c *C) {
tk.MustExec("DROP DATABASE IF EXISTS alterdb1")
tk.MustExec("CREATE DATABASE alterdb1 CHARSET=utf8 COLLATE=utf8_unicode_ci")

// No default DB error
// No default DB errors.
noDBFailedCases := []struct {
stmt string
errMsg string
Expand All @@ -1664,7 +1664,7 @@ func (s *testIntegrationSuite11) TestChangingDBCharset(c *C) {
}

verifyDBCharsetAndCollate := func(dbName, chs string, coll string) {
// check `SHOW CREATE SCHEMA`
// check `SHOW CREATE SCHEMA`.
r := tk.MustQuery("SHOW CREATE SCHEMA " + dbName).Rows()[0][1].(string)
c.Assert(strings.Contains(r, "CHARACTER SET "+chs), IsTrue)

Expand Down
8 changes: 4 additions & 4 deletions ddl/ddl_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func (d *ddl) CreateSchema(ctx sessionctx.Context, schema model.CIStr, charsetIn
return errors.Trace(err)
}
func (d *ddl) AlterSchema(ctx sessionctx.Context, stmt *ast.AlterDatabaseStmt) (err error) {
// Resolve target charset and collation from options
// Resolve target charset and collation from options.
var toCharset, toCollate string
for _, val := range stmt.Options {
switch val.Tp {
Expand Down Expand Up @@ -120,7 +120,7 @@ func (d *ddl) AlterSchema(ctx sessionctx.Context, stmt *ast.AlterDatabaseStmt) (
}
}

// Check if need change charset/collation
// Check if need to change charset/collation.
dbName := model.NewCIStr(stmt.Name)
is := d.GetInfoSchemaWithInterceptor(ctx)
dbInfo, ok := is.SchemaByName(dbName)
Expand All @@ -131,12 +131,12 @@ func (d *ddl) AlterSchema(ctx sessionctx.Context, stmt *ast.AlterDatabaseStmt) (
return nil
}

// check current TiDB limitations
// Check the current TiDB limitations.
if err = modifiableCharsetAndCollation(toCharset, toCollate, dbInfo.Charset, dbInfo.Collate); err != nil {
return errors.Trace(err)
}

// Do DDL job
// Do the DDL job.
job := &model.Job{
SchemaID: dbInfo.ID,
Type: model.ActionModifySchemaCharsetAndCollate,
Expand Down
7 changes: 6 additions & 1 deletion ddl/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,12 @@ func onModifySchemaCharsetAndCollate(t *meta.Meta, job *model.Job) (ver int64, _

dbInfo, err := t.GetDatabase(job.SchemaID)
if err != nil {
return nil, errors.Trace(err)
return ver, errors.Trace(err)
}

if dbInfo.Charset == toCharset && dbInfo.Collate == toCollate {
job.FinishDBJob(model.JobStateDone, model.StatePublic, ver, dbInfo)
return ver, nil
}

dbInfo.Charset = toCharset
Expand Down
9 changes: 1 addition & 8 deletions infoschema/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,18 +135,11 @@ func (b *Builder) applyModifySchemaCharsetAndCollate(m *meta.Meta, diff *model.S
return errors.Trace(err)
}
if di == nil {
// When we apply an old schema diff, the database may has been dropped already
// This should never happen.
return ErrDatabaseNotExists.GenWithStackByArgs(
fmt.Sprintf("(Schema ID %d)", diff.SchemaID),
)
}
oldInfo, ok := b.is.SchemaByName(di.Name)
if !ok {
return ErrDatabaseNotExists.GenWithStackByArgs(di.Name.O)
}
if oldInfo.Charset == di.Charset && oldInfo.Collate == di.Collate {
return nil
}
newDbInfo := b.copySchemaTables(di.Name.O)
newDbInfo.Charset = di.Charset
newDbInfo.Collate = di.Collate
Expand Down
2 changes: 1 addition & 1 deletion planner/core/preprocess.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ func (p *preprocessor) checkCreateDatabaseGrammar(stmt *ast.CreateDatabaseStmt)
}

func (p *preprocessor) checkAlterDatabaseGrammar(stmt *ast.AlterDatabaseStmt) {
// for 'ALTER DATABASE' statement, database name can be empty to alter default database
// for 'ALTER DATABASE' statement, database name can be empty to alter default database.
if isIncorrectName(stmt.Name) && !stmt.AlterDefaultDatabase {
p.err = ddl.ErrWrongDBName.GenWithStackByArgs(stmt.Name)
}
Expand Down

0 comments on commit 62cb0a1

Please sign in to comment.