From f1861f95f939ca7e0eec610c863afb7b49e1646a Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Wed, 2 Jun 2021 17:37:45 +0800 Subject: [PATCH 1/3] bindinfo,planner: report error when creating sql binding on temporary table --- bindinfo/bind_test.go | 10 +++++ planner/core/preprocess.go | 78 ++++++++++++++++++++++++++++++++------ 2 files changed, 77 insertions(+), 11 deletions(-) diff --git a/bindinfo/bind_test.go b/bindinfo/bind_test.go index 4175ddce77eb9..c73d8141968a9 100644 --- a/bindinfo/bind_test.go +++ b/bindinfo/bind_test.go @@ -30,6 +30,7 @@ import ( "github.com/pingcap/parser/terror" "github.com/pingcap/tidb/bindinfo" "github.com/pingcap/tidb/domain" + "github.com/pingcap/tidb/errno" "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/meta/autoid" "github.com/pingcap/tidb/metrics" @@ -2098,3 +2099,12 @@ func (s *testSuite) TestBindingWithoutCharset(c *C) { c.Assert(rows[0][0], Equals, "select * from `test` . `t` where `a` = ?") c.Assert(rows[0][1], Equals, "SELECT * FROM `test`.`t` WHERE `a` = 'aa'") } + +func (s *testSuite) TestTemporaryTable(c *C) { + tk := testkit.NewTestKit(c, s.store) + s.cleanBindingEnv(tk) + tk.MustExec("use test") + tk.MustExec("drop table if exists t") + tk.MustExec("create global temporary table t(a int, b int, key(a), key(b)) on commit delete rows") + tk.MustGetErrCode("create session binding for select * from t where b = 123 using select * from t ignore index(b) where b = 123;", errno.ErrOptOnTemporaryTable) +} diff --git a/planner/core/preprocess.go b/planner/core/preprocess.go index 947b1fdcef1ec..764a7de092173 100644 --- a/planner/core/preprocess.go +++ b/planner/core/preprocess.go @@ -24,6 +24,7 @@ import ( "github.com/pingcap/parser/format" "github.com/pingcap/parser/model" "github.com/pingcap/parser/mysql" + "github.com/pingcap/parser/terror" "github.com/pingcap/tidb/ddl" "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/expression" @@ -31,6 +32,7 @@ import ( "github.com/pingcap/tidb/meta/autoid" "github.com/pingcap/tidb/privilege" "github.com/pingcap/tidb/sessionctx" + "github.com/pingcap/tidb/table" "github.com/pingcap/tidb/types" driver "github.com/pingcap/tidb/types/parser_driver" "github.com/pingcap/tidb/util" @@ -335,6 +337,37 @@ func bindableStmtType(node ast.StmtNode) byte { return TypeInvalid } +func (p *preprocessor) tableByName(tn *ast.TableName) (table.Table, error) { + currentDB := p.ctx.GetSessionVars().CurrentDB + if tn.Schema.String() != "" { + currentDB = tn.Schema.L + } + if currentDB == "" { + return nil, errors.Trace(ErrNoDB) + } + sName := model.NewCIStr(currentDB) + tbl, err := p.ensureInfoSchema().TableByName(sName, tn.Name) + if err != nil { + // We should never leak that the table doesn't exist (i.e. attach ErrTableNotExists) + // unless we know that the user has permissions to it, should it exist. + // By checking here, this makes all SELECT/SHOW/INSERT/UPDATE/DELETE statements safe. + currentUser, activeRoles := p.ctx.GetSessionVars().User, p.ctx.GetSessionVars().ActiveRoles + if pm := privilege.GetPrivilegeManager(p.ctx); pm != nil { + if !pm.RequestVerification(activeRoles, sName.L, tn.Name.O, "", mysql.AllPrivMask) { + u := currentUser.Username + h := currentUser.Hostname + if currentUser.AuthHostname != "" { + u = currentUser.AuthUsername + h = currentUser.AuthHostname + } + return nil, ErrTableaccessDenied.GenWithStackByArgs(p.stmtType(), u, h, tn.Name.O) + } + } + return nil, err + } + return tbl, err +} + func (p *preprocessor) checkBindGrammar(originNode, hintedNode ast.StmtNode, defaultDB string) { origTp := bindableStmtType(originNode) hintedTp := bindableStmtType(hintedNode) @@ -353,6 +386,39 @@ func (p *preprocessor) checkBindGrammar(originNode, hintedNode ast.StmtNode, def return } } + + // Check the bind operation is not on any temporary table. + var resNode ast.ResultSetNode + switch n := originNode.(type) { + case *ast.SelectStmt: + resNode = n.From.TableRefs + case *ast.DeleteStmt: + resNode = n.TableRefs.TableRefs + case *ast.UpdateStmt: + resNode = n.TableRefs.TableRefs + case *ast.InsertStmt: + resNode = n.Table.TableRefs + } + if resNode != nil { + tblNames := extractTableList(resNode, nil, false) + for _, tn := range tblNames { + tbl, err := p.tableByName(tn) + if err != nil { + // If the operation is order is: drop table -> drop binding + // The table doesn't exist, it is not an error. + if terror.ErrorEqual(err, infoschema.ErrTableNotExists) { + continue + } + p.err = err + return + } + if tbl.Meta().TempTableType != model.TempTableNone { + p.err = ddl.ErrOptOnTemporaryTable.GenWithStackByArgs("create binding") + return + } + } + } + originSQL := parser.Normalize(utilparser.RestoreWithDefaultDB(originNode, defaultDB, originNode.Text())) hintedSQL := parser.Normalize(utilparser.RestoreWithDefaultDB(hintedNode, defaultDB, hintedNode.Text())) if originSQL != hintedSQL { @@ -596,17 +662,7 @@ func (p *preprocessor) checkDropDatabaseGrammar(stmt *ast.DropDatabaseStmt) { func (p *preprocessor) checkAdminCheckTableGrammar(stmt *ast.AdminStmt) { for _, table := range stmt.Tables { - currentDB := p.ctx.GetSessionVars().CurrentDB - if table.Schema.String() != "" { - currentDB = table.Schema.L - } - if currentDB == "" { - p.err = errors.Trace(ErrNoDB) - return - } - sName := model.NewCIStr(currentDB) - tName := table.Name - tableInfo, err := p.ensureInfoSchema().TableByName(sName, tName) + tableInfo, err := p.tableByName(table) if err != nil { p.err = err return From faf1eef638bd1195951dce4fef6ed540e4bcf775 Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Fri, 11 Jun 2021 17:04:42 +0800 Subject: [PATCH 2/3] address comment --- bindinfo/bind_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/bindinfo/bind_test.go b/bindinfo/bind_test.go index c73d8141968a9..13ea3e8f9d367 100644 --- a/bindinfo/bind_test.go +++ b/bindinfo/bind_test.go @@ -2105,6 +2105,12 @@ func (s *testSuite) TestTemporaryTable(c *C) { s.cleanBindingEnv(tk) tk.MustExec("use test") tk.MustExec("drop table if exists t") + tk.MustExec("set tidb_enable_global_temporary_table = true") tk.MustExec("create global temporary table t(a int, b int, key(a), key(b)) on commit delete rows") + tk.MustExec("create table t2(a int, b int, key(a), key(b))") tk.MustGetErrCode("create session binding for select * from t where b = 123 using select * from t ignore index(b) where b = 123;", errno.ErrOptOnTemporaryTable) + tk.MustGetErrCode("create binding for insert into t select * from t2 where t2.b = 1 and t2.c > 1 using insert into t select /*+ use_index(t2,c) */ * from t2 where t2.b = 1 and t2.c > 1", errno.ErrOptOnTemporaryTable) + tk.MustGetErrCode("create binding for replace into t select * from t2 where t2.b = 1 and t2.c > 1 using replace into t select /*+ use_index(t2,c) */ * from t2 where t2.b = 1 and t2.c > 1", errno.ErrOptOnTemporaryTable) + tk.MustGetErrCode("create binding for update t set a = 1 where b = 1 and c > 1 using update /*+ use_index(t, c) */ t set a = 1 where b = 1 and c > 1", errno.ErrOptOnTemporaryTable) + tk.MustGetErrCode("create binding for delete from t where b = 1 and c > 1 using delete /*+ use_index(t, c) */ from t where b = 1 and c > 1", errno.ErrOptOnTemporaryTable) } From cc1701a556a82076472f1ed68190d16a2aa9a0f8 Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Tue, 15 Jun 2021 13:21:53 +0800 Subject: [PATCH 3/3] address comment --- planner/core/preprocess.go | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/planner/core/preprocess.go b/planner/core/preprocess.go index a5e9ae3138781..a2f93454382aa 100644 --- a/planner/core/preprocess.go +++ b/planner/core/preprocess.go @@ -1301,27 +1301,12 @@ func (p *preprocessor) handleTableName(tn *ast.TableName) { return } - table, err := p.ensureInfoSchema().TableByName(tn.Schema, tn.Name) + table, err := p.tableByName(tn) if err != nil { - // We should never leak that the table doesn't exist (i.e. attach ErrTableNotExists) - // unless we know that the user has permissions to it, should it exist. - // By checking here, this makes all SELECT/SHOW/INSERT/UPDATE/DELETE statements safe. - currentUser, activeRoles := p.ctx.GetSessionVars().User, p.ctx.GetSessionVars().ActiveRoles - if pm := privilege.GetPrivilegeManager(p.ctx); pm != nil { - if !pm.RequestVerification(activeRoles, tn.Schema.L, tn.Name.O, "", mysql.AllPrivMask) { - u := currentUser.Username - h := currentUser.Hostname - if currentUser.AuthHostname != "" { - u = currentUser.AuthUsername - h = currentUser.AuthHostname - } - p.err = ErrTableaccessDenied.GenWithStackByArgs(p.stmtType(), u, h, tn.Name.O) - return - } - } p.err = err return } + tableInfo := table.Meta() dbInfo, _ := p.ensureInfoSchema().SchemaByName(tn.Schema) // tableName should be checked as sequence object.