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

planner: check view recursion when building source from view (#20398) #21001

Merged
merged 5 commits into from
Nov 17, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions errors.toml
Original file line number Diff line number Diff line change
Expand Up @@ -761,6 +761,11 @@ error = '''
View '%-.192s.%-.192s' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them
'''

["planner:1462"]
error = '''
`%-.192s`.`%-.192s` contains view recursion
'''

["planner:1747"]
error = '''
PARTITION () clause on non partitioned table
Expand Down
13 changes: 13 additions & 0 deletions executor/ddl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,19 @@ func (s *testSuite6) TestCreateView(c *C) {
tk.MustExec("drop view v_nested, v_nested2")
}

func (s *testSuite6) TestViewRecursion(c *C) {
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test")
tk.MustExec("create table if not exists t(a int)")
tk.MustExec("create definer='root'@'localhost' view recursive_view1 as select * from t")
tk.MustExec("create definer='root'@'localhost' view recursive_view2 as select * from recursive_view1")
tk.MustExec("drop table t")
tk.MustExec("rename table recursive_view2 to t")
_, err := tk.Exec("select * from recursive_view1")
c.Assert(terror.ErrorEqual(err, plannercore.ErrViewRecursive), IsTrue)
tk.MustExec("drop view recursive_view1, t")
}

func (s *testSuite6) TestIssue16250(c *C) {
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test")
Expand Down
1 change: 1 addition & 0 deletions planner/core/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ var (
ErrUnknown = terror.ClassOptimizer.New(mysql.ErrUnknown, mysql.MySQLErrName[mysql.ErrUnknown])
ErrUnknownTable = terror.ClassOptimizer.New(mysql.ErrUnknownTable, mysql.MySQLErrName[mysql.ErrUnknownTable])
ErrNoSuchTable = terror.ClassOptimizer.New(mysql.ErrNoSuchTable, mysql.MySQLErrName[mysql.ErrNoSuchTable])
ErrViewRecursive = terror.ClassOptimizer.New(mysql.ErrViewRecursive, mysql.MySQLErrName[mysql.ErrViewRecursive])
ErrWrongArguments = terror.ClassOptimizer.New(mysql.ErrWrongArguments, mysql.MySQLErrName[mysql.ErrWrongArguments])
ErrWrongNumberOfColumnsInSelect = terror.ClassOptimizer.New(mysql.ErrWrongNumberOfColumnsInSelect, mysql.MySQLErrName[mysql.ErrWrongNumberOfColumnsInSelect])
ErrBadGeneratedColumn = terror.ClassOptimizer.New(mysql.ErrBadGeneratedColumn, mysql.MySQLErrName[mysql.ErrBadGeneratedColumn])
Expand Down
35 changes: 31 additions & 4 deletions planner/core/logical_plan_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"github.com/pingcap/parser/model"
"github.com/pingcap/parser/mysql"
"github.com/pingcap/parser/opcode"
"github.com/pingcap/parser/terror"
"github.com/pingcap/tidb/domain"
"github.com/pingcap/tidb/expression"
"github.com/pingcap/tidb/expression/aggregation"
Expand All @@ -52,6 +53,7 @@ import (
"github.com/pingcap/tidb/util/chunk"
utilhint "github.com/pingcap/tidb/util/hint"
"github.com/pingcap/tidb/util/plancodec"
"github.com/pingcap/tidb/util/set"
)

const (
Expand Down Expand Up @@ -2744,9 +2746,6 @@ func (b *PlanBuilder) buildDataSource(ctx context.Context, tn *ast.TableName, as
}

if tableInfo.IsView() {
if b.capFlag&collectUnderlyingViewName != 0 {
b.underlyingViewNames.Insert(dbName.L + "." + tn.Name.L)
}
return b.BuildDataSourceFromView(ctx, dbName, tableInfo)
}

Expand Down Expand Up @@ -3068,8 +3067,33 @@ func (b *PlanBuilder) buildMemTable(_ context.Context, dbName model.CIStr, table
return p, nil
}

// checkRecursiveView checks whether this view is recursively defined.
func (b *PlanBuilder) checkRecursiveView(dbName model.CIStr, tableName model.CIStr) (func(), error) {
viewFullName := dbName.L + "." + tableName.L
if b.buildingViewStack == nil {
b.buildingViewStack = set.NewStringSet()
}
// If this view has already been on the building stack, it means
// this view contains a recursive definition.
if b.buildingViewStack.Exist(viewFullName) {
return nil, ErrViewRecursive.GenWithStackByArgs(dbName.O, tableName.O)
}
// If the view is being renamed, we return the mysql compatible error message.
if b.capFlag&renameView != 0 && viewFullName == b.renamingViewName {
return nil, ErrNoSuchTable.GenWithStackByArgs(dbName.O, tableName.O)
}
b.buildingViewStack.Insert(viewFullName)
return func() { delete(b.buildingViewStack, viewFullName) }, nil
}

// BuildDataSourceFromView is used to build LogicalPlan from view
func (b *PlanBuilder) BuildDataSourceFromView(ctx context.Context, dbName model.CIStr, tableInfo *model.TableInfo) (LogicalPlan, error) {
deferFunc, err := b.checkRecursiveView(dbName, tableInfo.Name)
if err != nil {
return nil, err
}
defer deferFunc()

charset, collation := b.ctx.GetSessionVars().GetCharsetInfo()
viewParser := parser.New()
viewParser.EnableWindowFunc(b.ctx.GetSessionVars().EnableWindowFunction)
Expand All @@ -3081,7 +3105,10 @@ func (b *PlanBuilder) BuildDataSourceFromView(ctx context.Context, dbName model.
b.visitInfo = make([]visitInfo, 0)
selectLogicalPlan, err := b.Build(ctx, selectNode)
if err != nil {
err = ErrViewInvalid.GenWithStackByArgs(dbName.O, tableInfo.Name.O)
if terror.ErrorNotEqual(err, ErrViewRecursive) &&
terror.ErrorNotEqual(err, ErrNoSuchTable) {
err = ErrViewInvalid.GenWithStackByArgs(dbName.O, tableInfo.Name.O)
}
return nil, err
}

Expand Down
22 changes: 10 additions & 12 deletions planner/core/planbuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,9 +392,9 @@ const (
// canExpandAST indicates whether the origin AST can be expanded during plan
// building. ONLY used for `CreateViewStmt` now.
canExpandAST
// collectUnderlyingViewName indicates whether to collect the underlying
// view names of a CreateViewStmt during plan building.
collectUnderlyingViewName
// renameView indicates a view is being renamed, so we cannot use the origin
// definition of that view.
renameView
)

// PlanBuilder builds Plan from an ast.Node.
Expand Down Expand Up @@ -444,8 +444,10 @@ type PlanBuilder struct {

// SelectLock need this information to locate the lock on partitions.
partitionedTable []table.PartitionedTable
// CreateView needs this information to check whether exists nested view.
underlyingViewNames set.StringSet
// buildingViewStack is used to check whether there is a recursive view.
buildingViewStack set.StringSet
// renamingViewName is the name of the view which is being renamed.
renamingViewName string
}

type handleColHelper struct {
Expand Down Expand Up @@ -2777,20 +2779,16 @@ func (b *PlanBuilder) buildDDL(ctx context.Context, node ast.DDLNode) (Plan, err
v.ReferTable.Name.L, "", authErr)
}
case *ast.CreateViewStmt:
b.capFlag |= canExpandAST
b.capFlag |= collectUnderlyingViewName
b.capFlag |= canExpandAST | renameView
b.renamingViewName = v.ViewName.Schema.L + "." + v.ViewName.Name.L
defer func() {
b.capFlag &= ^canExpandAST
b.capFlag &= ^collectUnderlyingViewName
b.capFlag &= ^renameView
}()
b.underlyingViewNames = set.NewStringSet()
plan, err := b.Build(ctx, v.Select)
if err != nil {
return nil, err
}
if b.underlyingViewNames.Exist(v.ViewName.Schema.L + "." + v.ViewName.Name.L) {
return nil, ErrNoSuchTable.GenWithStackByArgs(v.ViewName.Schema.O, v.ViewName.Name.O)
}
schema := plan.Schema()
names := plan.OutputNames()
if v.Cols == nil {
Expand Down