From 466604232de6af6cfae15e00bcbc6a9dd18a3e5e Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Mon, 24 May 2021 14:36:15 +0800 Subject: [PATCH 1/6] executor,table: fix 'show create table' for the temporary table --- executor/show.go | 13 ++++++++++++- executor/show_test.go | 22 ++++++++++++++++++++++ table/tables/tables.go | 7 +++++-- 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/executor/show.go b/executor/show.go index 09e3d0c71e7b4..c3eb95233cc06 100644 --- a/executor/show.go +++ b/executor/show.go @@ -775,7 +775,13 @@ func ConstructResultOfShowCreateTable(ctx sessionctx.Context, tableInfo *model.T } sqlMode := ctx.GetSessionVars().SQLMode - fmt.Fprintf(buf, "CREATE TABLE %s (\n", stringutil.Escape(tableInfo.Name.O, sqlMode)) + tableName := stringutil.Escape(tableInfo.Name.O, sqlMode) + switch tableInfo.TempTableType { + case model.TempTableGlobal: + fmt.Fprintf(buf, "CREATE /* GLOBAL TEMPORARY */ TABLE %s (\n", tableName) + default: + fmt.Fprintf(buf, "CREATE TABLE %s (\n", tableName) + } var pkCol *model.ColumnInfo var hasAutoIncID bool needAddComma := false @@ -1009,6 +1015,11 @@ func ConstructResultOfShowCreateTable(ctx sessionctx.Context, tableInfo *model.T if len(tableInfo.Comment) > 0 { fmt.Fprintf(buf, " COMMENT='%s'", format.OutputFormat(tableInfo.Comment)) } + + if tableInfo.TempTableType == model.TempTableGlobal { + fmt.Fprintf(buf, " /* ON COMMIT DELETE ROWS */") + } + // add partition info here. appendPartitionInfo(tableInfo.Partition, buf) return nil diff --git a/executor/show_test.go b/executor/show_test.go index ea6d6734159b6..07dad141e5b62 100644 --- a/executor/show_test.go +++ b/executor/show_test.go @@ -1303,3 +1303,25 @@ func (s *testSuite5) TestShowPerformanceSchema(c *C) { testkit.Rows("events_statements_summary_by_digest 0 SCHEMA_NAME 1 SCHEMA_NAME A 0 YES BTREE YES NULL NO", "events_statements_summary_by_digest 0 SCHEMA_NAME 2 DIGEST A 0 YES BTREE YES NULL NO")) } + +func (s *testSuite5) TestShowTemporaryTable(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec("create global temporary table t1 (id int) on commit delete rows") + tk.MustExec("create global temporary table t3 (i int primary key, j int) on commit delete rows") + // For issue https://github.com/pingcap/tidb/issues/24752 + tk.MustQuery("show create table t1").Check(testkit.Rows("t1 CREATE /* GLOBAL TEMPORARY */ TABLE `t1` (\n" + + " `id` int(11) DEFAULT NULL\n" + + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin /* ON COMMIT DELETE ROWS */")) + // No panic, fix issue https://github.com/pingcap/tidb/issues/24788 + expect := "CREATE /* GLOBAL TEMPORARY */ TABLE `t3` (\n" + + " `i` int(11) NOT NULL,\n" + + " `j` int(11) DEFAULT NULL,\n" + + " PRIMARY KEY (`i`) /*T![clustered_index] CLUSTERED */\n" + + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin /* ON COMMIT DELETE ROWS */" + tk.MustQuery("show create table t3").Check(testkit.Rows("t3 " + expect)) + + // Verify that the `show create table` result can be used to build the table. + createTable := strings.ReplaceAll(expect, "t3", "t4") + tk.MustExec(createTable) +} diff --git a/table/tables/tables.go b/table/tables/tables.go index 37e6ab1103a89..b74314bae0495 100644 --- a/table/tables/tables.go +++ b/table/tables/tables.go @@ -1374,8 +1374,11 @@ func (t *TableCommon) Allocators(ctx sessionctx.Context) autoid.Allocators { } else if ctx.GetSessionVars().IDAllocator == nil { // Use an independent allocator for global temporary tables. if t.meta.TempTableType == model.TempTableGlobal { - alloc := ctx.GetSessionVars().GetTemporaryTable(t.meta).GetAutoIDAllocator() - return autoid.Allocators{alloc} + if alloc := ctx.GetSessionVars().GetTemporaryTable(t.meta).GetAutoIDAllocator(); alloc != nil { + return autoid.Allocators{alloc} + } + // If the session is not in a txn, for example, in "show create table", use the original allocator. + // Otherwise the would be a nil pointer dereference, } return t.allocs } From 134cd6f5ca15b600a776676d6400aa2b27e1f66f Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Tue, 25 May 2021 17:27:46 +0800 Subject: [PATCH 2/6] address comment --- ddl/ddl_api.go | 2 +- executor/show.go | 4 ++-- executor/show_test.go | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 2cfd3f524a29f..1d19dc944ebbd 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -2225,7 +2225,7 @@ func handleTableOptions(options []*ast.TableOption, tbInfo *model.TableInfo) err // We don't handle charset and collate here since they're handled in `getCharsetAndCollateInTableOption`. case ast.TableOptionEngine: if tbInfo.TempTableType != model.TempTableNone { - if op.StrValue != "" && !strings.EqualFold(op.StrValue, "memory") { + if op.StrValue != "" && !strings.EqualFold(op.StrValue, "memory") && !strings.EqualFold(op.StrValue, "innodb") { return errors.Trace(errUnsupportedEngineTemporary) } } diff --git a/executor/show.go b/executor/show.go index c3eb95233cc06..7463eed06173d 100644 --- a/executor/show.go +++ b/executor/show.go @@ -778,7 +778,7 @@ func ConstructResultOfShowCreateTable(ctx sessionctx.Context, tableInfo *model.T tableName := stringutil.Escape(tableInfo.Name.O, sqlMode) switch tableInfo.TempTableType { case model.TempTableGlobal: - fmt.Fprintf(buf, "CREATE /* GLOBAL TEMPORARY */ TABLE %s (\n", tableName) + fmt.Fprintf(buf, "CREATE GLOBAL TEMPORARY TABLE %s (\n", tableName) default: fmt.Fprintf(buf, "CREATE TABLE %s (\n", tableName) } @@ -1017,7 +1017,7 @@ func ConstructResultOfShowCreateTable(ctx sessionctx.Context, tableInfo *model.T } if tableInfo.TempTableType == model.TempTableGlobal { - fmt.Fprintf(buf, " /* ON COMMIT DELETE ROWS */") + fmt.Fprintf(buf, " ON COMMIT DELETE ROWS") } // add partition info here. diff --git a/executor/show_test.go b/executor/show_test.go index 07dad141e5b62..283f1fbea87d6 100644 --- a/executor/show_test.go +++ b/executor/show_test.go @@ -1310,15 +1310,15 @@ func (s *testSuite5) TestShowTemporaryTable(c *C) { tk.MustExec("create global temporary table t1 (id int) on commit delete rows") tk.MustExec("create global temporary table t3 (i int primary key, j int) on commit delete rows") // For issue https://github.com/pingcap/tidb/issues/24752 - tk.MustQuery("show create table t1").Check(testkit.Rows("t1 CREATE /* GLOBAL TEMPORARY */ TABLE `t1` (\n" + + tk.MustQuery("show create table t1").Check(testkit.Rows("t1 CREATE GLOBAL TEMPORARY TABLE `t1` (\n" + " `id` int(11) DEFAULT NULL\n" + - ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin /* ON COMMIT DELETE ROWS */")) + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin ON COMMIT DELETE ROWS")) // No panic, fix issue https://github.com/pingcap/tidb/issues/24788 - expect := "CREATE /* GLOBAL TEMPORARY */ TABLE `t3` (\n" + + expect := "CREATE GLOBAL TEMPORARY TABLE `t3` (\n" + " `i` int(11) NOT NULL,\n" + " `j` int(11) DEFAULT NULL,\n" + " PRIMARY KEY (`i`) /*T![clustered_index] CLUSTERED */\n" + - ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin /* ON COMMIT DELETE ROWS */" + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin ON COMMIT DELETE ROWS" tk.MustQuery("show create table t3").Check(testkit.Rows("t3 " + expect)) // Verify that the `show create table` result can be used to build the table. From 4efbca104c386f04830c82729481f68797a4914a Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Tue, 25 May 2021 17:28:53 +0800 Subject: [PATCH 3/6] address comment --- table/tables/tables.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/table/tables/tables.go b/table/tables/tables.go index b74314bae0495..5782b90543654 100644 --- a/table/tables/tables.go +++ b/table/tables/tables.go @@ -1378,7 +1378,7 @@ func (t *TableCommon) Allocators(ctx sessionctx.Context) autoid.Allocators { return autoid.Allocators{alloc} } // If the session is not in a txn, for example, in "show create table", use the original allocator. - // Otherwise the would be a nil pointer dereference, + // Otherwise the would be a nil pointer dereference. } return t.allocs } From 5ab9c23ee35caf9cc16dc6fd75f763d20dec7b18 Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Tue, 25 May 2021 18:15:51 +0800 Subject: [PATCH 4/6] fix CI --- ddl/db_integration_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ddl/db_integration_test.go b/ddl/db_integration_test.go index 9fe96a50afa02..7440d4933725c 100644 --- a/ddl/db_integration_test.go +++ b/ddl/db_integration_test.go @@ -2784,8 +2784,7 @@ func (s *testIntegrationSuite3) TestCreateTemporaryTable(c *C) { // Not support yet. tk.MustGetErrCode("create global temporary table t (id int) on commit preserve rows", errno.ErrUnsupportedDDLOperation) - // Engine type can only be 'memory' or empty for now. - tk.MustGetErrCode("create global temporary table t (id int) engine = 'innodb' on commit delete rows", errno.ErrUnsupportedDDLOperation) + tk.MustGetErrCode("create global temporary table t (id int) engine = 'myisam' on commit delete rows", errno.ErrUnsupportedDDLOperation) // Follow the behaviour of the old version TiDB: parse and ignore the 'temporary' keyword. tk.MustGetErrCode("create temporary table t(id int)", errno.ErrNotSupportedYet) From 4afaeeda3799d8f056d941ebf1322e9dac308204 Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Tue, 25 May 2021 19:44:17 +0800 Subject: [PATCH 5/6] address comment --- ddl/db_integration_test.go | 3 ++- executor/show.go | 9 ++++++++- executor/show_test.go | 4 ++-- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/ddl/db_integration_test.go b/ddl/db_integration_test.go index 7440d4933725c..9fe96a50afa02 100644 --- a/ddl/db_integration_test.go +++ b/ddl/db_integration_test.go @@ -2784,7 +2784,8 @@ func (s *testIntegrationSuite3) TestCreateTemporaryTable(c *C) { // Not support yet. tk.MustGetErrCode("create global temporary table t (id int) on commit preserve rows", errno.ErrUnsupportedDDLOperation) - tk.MustGetErrCode("create global temporary table t (id int) engine = 'myisam' on commit delete rows", errno.ErrUnsupportedDDLOperation) + // Engine type can only be 'memory' or empty for now. + tk.MustGetErrCode("create global temporary table t (id int) engine = 'innodb' on commit delete rows", errno.ErrUnsupportedDDLOperation) // Follow the behaviour of the old version TiDB: parse and ignore the 'temporary' keyword. tk.MustGetErrCode("create temporary table t(id int)", errno.ErrNotSupportedYet) diff --git a/executor/show.go b/executor/show.go index 7463eed06173d..78f2e409b27e4 100644 --- a/executor/show.go +++ b/executor/show.go @@ -958,7 +958,14 @@ func ConstructResultOfShowCreateTable(ctx sessionctx.Context, tableInfo *model.T buf.WriteString("\n") - buf.WriteString(") ENGINE=InnoDB") + switch tableInfo.TempTableType { + case model.TempTableNone: + buf.WriteString(") ENGINE=InnoDB") + default: + // For now the only supported engine for temporary table is memory. + buf.WriteString(") ENGINE=memory") + } + // We need to explicitly set the default charset and collation // to make it work on MySQL server which has default collate utf8_general_ci. if len(tblCollate) == 0 || tblCollate == "binary" { diff --git a/executor/show_test.go b/executor/show_test.go index 283f1fbea87d6..57cec97990dc4 100644 --- a/executor/show_test.go +++ b/executor/show_test.go @@ -1312,13 +1312,13 @@ func (s *testSuite5) TestShowTemporaryTable(c *C) { // For issue https://github.com/pingcap/tidb/issues/24752 tk.MustQuery("show create table t1").Check(testkit.Rows("t1 CREATE GLOBAL TEMPORARY TABLE `t1` (\n" + " `id` int(11) DEFAULT NULL\n" + - ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin ON COMMIT DELETE ROWS")) + ") ENGINE=memory DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin ON COMMIT DELETE ROWS")) // No panic, fix issue https://github.com/pingcap/tidb/issues/24788 expect := "CREATE GLOBAL TEMPORARY TABLE `t3` (\n" + " `i` int(11) NOT NULL,\n" + " `j` int(11) DEFAULT NULL,\n" + " PRIMARY KEY (`i`) /*T![clustered_index] CLUSTERED */\n" + - ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin ON COMMIT DELETE ROWS" + ") ENGINE=memory DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin ON COMMIT DELETE ROWS" tk.MustQuery("show create table t3").Check(testkit.Rows("t3 " + expect)) // Verify that the `show create table` result can be used to build the table. From b3b96e2cb66d11b982ebfa115f320cbc91726a6b Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Tue, 25 May 2021 19:54:07 +0800 Subject: [PATCH 6/6] address comment --- ddl/ddl_api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 1d19dc944ebbd..2cfd3f524a29f 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -2225,7 +2225,7 @@ func handleTableOptions(options []*ast.TableOption, tbInfo *model.TableInfo) err // We don't handle charset and collate here since they're handled in `getCharsetAndCollateInTableOption`. case ast.TableOptionEngine: if tbInfo.TempTableType != model.TempTableNone { - if op.StrValue != "" && !strings.EqualFold(op.StrValue, "memory") && !strings.EqualFold(op.StrValue, "innodb") { + if op.StrValue != "" && !strings.EqualFold(op.StrValue, "memory") { return errors.Trace(errUnsupportedEngineTemporary) } }