diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go index b4ee74340410..b046e798a837 100644 --- a/pkg/sql/alter_table.go +++ b/pkg/sql/alter_table.go @@ -192,12 +192,13 @@ func (n *alterTableNode) Start() error { if !found { continue } - err := n.p.canRemoveDependentViewGeneric("column", string(t.Column), ref, t.DropBehavior) + err := n.p.canRemoveDependentViewGeneric( + "column", string(t.Column), n.tableDesc.ParentID, ref, t.DropBehavior) if err != nil { return err } - viewDesc, err := - n.p.getViewDescForCascade("column", string(t.Column), ref.ID, t.DropBehavior) + viewDesc, err := n.p.getViewDescForCascade( + "column", string(t.Column), n.tableDesc.ParentID, ref.ID, t.DropBehavior) if err != nil { return err } @@ -244,13 +245,13 @@ func (n *alterTableNode) Start() error { if t.IfExists { continue } - return errors.Errorf("constraint %q does not exist", t.Constraint) + return fmt.Errorf("constraint %q does not exist", t.Constraint) } switch details.Kind { case sqlbase.ConstraintTypePK: - return errors.Errorf("cannot drop primary key") + return fmt.Errorf("cannot drop primary key") case sqlbase.ConstraintTypeUnique: - return errors.Errorf("UNIQUE constraint depends on index %q, use DROP INDEX if you really want to drop it", t.Constraint) + return fmt.Errorf("UNIQUE constraint depends on index %q, use DROP INDEX if you really want to drop it", t.Constraint) case sqlbase.ConstraintTypeCheck: for i := range n.tableDesc.Checks { if n.tableDesc.Checks[i].Name == name { @@ -282,7 +283,7 @@ func (n *alterTableNode) Start() error { name := string(t.Constraint) constraint, ok := info[name] if !ok { - return errors.Errorf("constraint %q does not exist", t.Constraint) + return fmt.Errorf("constraint %q does not exist", t.Constraint) } if !constraint.Unvalidated { continue diff --git a/pkg/sql/create.go b/pkg/sql/create.go index eb8e3410e238..1f507e4d8424 100644 --- a/pkg/sql/create.go +++ b/pkg/sql/create.go @@ -59,7 +59,7 @@ func (p *planner) CreateDatabase(n *parser.CreateDatabase) (planNode, error) { } if p.session.User != security.RootUser { - return nil, errors.Errorf("only %s is allowed to create databases", security.RootUser) + return nil, fmt.Errorf("only %s is allowed to create databases", security.RootUser) } return &createDatabaseNode{p: p, n: n}, nil @@ -663,7 +663,7 @@ func (p *planner) resolveFK( } if len(targetCols) != len(srcCols) { - return errors.Errorf("%d columns must reference exactly %d columns in referenced table (found %d)", + return fmt.Errorf("%d columns must reference exactly %d columns in referenced table (found %d)", len(srcCols), len(srcCols), len(targetCols)) } @@ -925,7 +925,7 @@ func (n *createViewNode) makeViewTableDesc( // TODO(a-robinson): Support star expressions as soon as we can (#10028). if planContainsStar(n.sourcePlan) { - return desc, errors.New("views do not currently support * expressions") + return desc, fmt.Errorf("views do not currently support * expressions") } n.resolveViewDependencies(&desc, affected) @@ -1148,7 +1148,8 @@ func (p *planner) makeTableDesc( if idx.ForeignKey.IsSet() { for i := range idx.ColumnIDs { if _, ok := colsInFKs[idx.ColumnIDs[i]]; ok { - return desc, errors.Errorf("column %q cannot be used by multiple foreign key constraints", idx.ColumnNames[i]) + return desc, fmt.Errorf( + "column %q cannot be used by multiple foreign key constraints", idx.ColumnNames[i]) } colsInFKs[idx.ColumnIDs[i]] = struct{}{} } diff --git a/pkg/sql/drop.go b/pkg/sql/drop.go index 6f4b1d5e3c2e..857472d9d621 100644 --- a/pkg/sql/drop.go +++ b/pkg/sql/drop.go @@ -325,12 +325,13 @@ func (n *dropIndexNode) Start() error { for _, tableRef := range tableDesc.DependedOnBy { if tableRef.IndexID == idx.ID { // Ensure that we have DROP privilege on all dependent views - err := n.p.canRemoveDependentViewGeneric("index", idx.Name, tableRef, n.n.DropBehavior) + err := n.p.canRemoveDependentViewGeneric( + "index", idx.Name, tableDesc.ParentID, tableRef, n.n.DropBehavior) if err != nil { return err } - viewDesc, err := - n.p.getViewDescForCascade("index", idx.Name, tableRef.ID, n.n.DropBehavior) + viewDesc, err := n.p.getViewDescForCascade( + "index", idx.Name, tableDesc.ParentID, tableRef.ID, n.n.DropBehavior) if err != nil { return err } @@ -628,16 +629,17 @@ func (p *planner) canRemoveDependentView( ref sqlbase.TableDescriptor_Reference, behavior parser.DropBehavior, ) error { - return p.canRemoveDependentViewGeneric(from.TypeName(), from.Name, ref, behavior) + return p.canRemoveDependentViewGeneric(from.TypeName(), from.Name, from.ParentID, ref, behavior) } func (p *planner) canRemoveDependentViewGeneric( typeName string, objName string, + parentID sqlbase.ID, ref sqlbase.TableDescriptor_Reference, behavior parser.DropBehavior, ) error { - viewDesc, err := p.getViewDescForCascade(typeName, objName, ref.ID, behavior) + viewDesc, err := p.getViewDescForCascade(typeName, objName, parentID, ref.ID, behavior) if err != nil { return err } @@ -798,8 +800,8 @@ func (p *planner) dropTableImpl(tableDesc *sqlbase.TableDescriptor) ([]string, e // Drop all views that depend on this table, assuming that we wouldn't have // made it to this point if `cascade` wasn't enabled. for _, ref := range tableDesc.DependedOnBy { - viewDesc, err := - p.getViewDescForCascade(tableDesc.TypeName(), tableDesc.Name, ref.ID, parser.DropCascade) + viewDesc, err := p.getViewDescForCascade( + tableDesc.TypeName(), tableDesc.Name, tableDesc.ParentID, ref.ID, parser.DropCascade) if err != nil { return droppedViews, err } @@ -920,8 +922,8 @@ func (p *planner) dropViewImpl( if behavior == parser.DropCascade { for _, ref := range viewDesc.DependedOnBy { - dependentDesc, err := - p.getViewDescForCascade(viewDesc.TypeName(), viewDesc.Name, ref.ID, behavior) + dependentDesc, err := p.getViewDescForCascade( + viewDesc.TypeName(), viewDesc.Name, viewDesc.ParentID, ref.ID, behavior) if err != nil { return cascadeDroppedViews, err } @@ -988,7 +990,7 @@ func removeMatchingReferences( } func (p *planner) getViewDescForCascade( - typeName string, objName string, viewID sqlbase.ID, behavior parser.DropBehavior, + typeName string, objName string, parentID, viewID sqlbase.ID, behavior parser.DropBehavior, ) (*sqlbase.TableDescriptor, error) { viewDesc, err := sqlbase.GetTableDescFromID(p.txn, viewID) if err != nil { @@ -996,12 +998,17 @@ func (p *planner) getViewDescForCascade( return nil, errors.Wrapf(err, "error resolving dependent view ID %d", viewID) } if behavior != parser.DropCascade { - viewName, err := p.getQualifiedTableName(viewDesc) - if err != nil { - log.Warningf(p.ctx(), "unable to retrieve qualified name of view %d: %v", viewID, err) - return nil, errors.Wrapf(err, "cannot drop %s %q because it is depended on by a view") + viewName := viewDesc.Name + if viewDesc.ParentID != parentID { + var err error + viewName, err = p.getQualifiedTableName(viewDesc) + if err != nil { + log.Warningf(p.ctx(), "unable to retrieve qualified name of view %d: %v", viewID, err) + return nil, sqlbase.NewDependentObjectError( + "cannot drop %s %q because a view depends on it", typeName, objName) + } } - return nil, errors.Errorf("cannot drop %s %q because it is depended on by view %q", + return nil, sqlbase.NewDependentObjectError("cannot drop %s %q because view %q depends on it", typeName, objName, viewName) } return viewDesc, nil diff --git a/pkg/sql/rename.go b/pkg/sql/rename.go index cda182017206..138afa86ac80 100644 --- a/pkg/sql/rename.go +++ b/pkg/sql/rename.go @@ -79,15 +79,23 @@ func (p *planner) RenameDatabase(n *parser.RenameDatabase) (planNode, error) { continue } if len(tbDesc.DependedOnBy) > 0 { - viewName, err := p.getQualifiedTableNameFromID(tbDesc.DependedOnBy[0].ID) + viewDesc, err := sqlbase.GetTableDescFromID(p.txn, tbDesc.DependedOnBy[0].ID) if err != nil { - log.Warningf(p.ctx(), "Unable to retrieve name of view %d: %v", - tbDesc.DependedOnBy[0].ID, err) - return nil, errors.Errorf( - "cannot rename database because table %q is depended on by a view", tbDesc.Name) + return nil, err } - return nil, errors.Errorf("cannot rename database because table %q is depended on by view %q", - tbDesc.Name, viewName) + viewName := viewDesc.Name + if dbDesc.ID != viewDesc.ParentID { + var err error + viewName, err = p.getQualifiedTableName(viewDesc) + if err != nil { + log.Warningf(p.ctx(), "Unable to retrieve fully-qualified name of view %d: %v", + viewDesc.ID, err) + return nil, sqlbase.NewDependentObjectError( + "cannot rename database because a view depends on table %q", tbDesc.Name) + } + } + return nil, sqlbase.NewDependentObjectError( + "cannot rename database because view %q depends on table %q", viewName, tbDesc.Name) } } @@ -167,7 +175,7 @@ func (p *planner) RenameTable(n *parser.RenameTable) (planNode, error) { // query with the new name, we simply disallow such renames for now. if len(tableDesc.DependedOnBy) > 0 { return nil, p.dependentViewRenameError( - tableDesc.TypeName(), oldTn.String(), tableDesc.DependedOnBy[0].ID) + tableDesc.TypeName(), oldTn.String(), tableDesc.ParentID, tableDesc.DependedOnBy[0].ID) } // Check if target database exists. @@ -272,7 +280,8 @@ func (p *planner) RenameIndex(n *parser.RenameIndex) (planNode, error) { if tableRef.IndexID != tableDesc.Indexes[i].ID { continue } - return nil, p.dependentViewRenameError("index", n.Index.Index.String(), tableRef.ID) + return nil, p.dependentViewRenameError( + "index", n.Index.Index.String(), tableDesc.ParentID, tableRef.ID) } if n.NewName == "" { @@ -362,7 +371,8 @@ func (p *planner) RenameColumn(n *parser.RenameColumn) (planNode, error) { } } if found { - return nil, p.dependentViewRenameError("column", n.Name.String(), tableRef.ID) + return nil, p.dependentViewRenameError( + "column", n.Name.String(), tableDesc.ParentID, tableRef.ID) } } @@ -429,13 +439,23 @@ func (p *planner) RenameColumn(n *parser.RenameColumn) (planNode, error) { // TODO(a-robinson): Support renaming objects depended on by views once we have // a better encoding for view queries (#10083). -func (p *planner) dependentViewRenameError(typeName, objName string, viewID sqlbase.ID) error { - viewName, err := p.getQualifiedTableNameFromID(viewID) +func (p *planner) dependentViewRenameError( + typeName, objName string, parentID, viewID sqlbase.ID, +) error { + viewDesc, err := sqlbase.GetTableDescFromID(p.txn, viewID) if err != nil { - log.Warningf(p.ctx(), "unable to retrieve name of view %d: %v", viewID, err) - return errors.Errorf("cannot rename %s %q because it is depended on by a view", - typeName, objName) + return err + } + viewName := viewDesc.Name + if viewDesc.ParentID != parentID { + var err error + viewName, err = p.getQualifiedTableName(viewDesc) + if err != nil { + log.Warningf(p.ctx(), "unable to retrieve name of view %d: %v", viewID, err) + return sqlbase.NewDependentObjectError("cannot rename %s %q because a view depends on it", + typeName, objName) + } } - return errors.Errorf("cannot rename %s %q because it is depended on by view %q", + return sqlbase.NewDependentObjectError("cannot rename %s %q because view %q depends on it", typeName, objName, viewName) } diff --git a/pkg/sql/sqlbase/errors.go b/pkg/sql/sqlbase/errors.go index be714ab49e7b..840dcaf3bf4c 100644 --- a/pkg/sql/sqlbase/errors.go +++ b/pkg/sql/sqlbase/errors.go @@ -60,6 +60,7 @@ var _ ErrorWithPGCode = &ErrDatabaseAlreadyExists{} var _ ErrorWithPGCode = &ErrRelationAlreadyExists{} var _ ErrorWithPGCode = &ErrWrongObjectType{} var _ ErrorWithPGCode = &ErrSyntax{} +var _ ErrorWithPGCode = &ErrDependentObject{} const ( txnAbortedMsg = "current transaction is aborted, commands ignored " + @@ -378,6 +379,31 @@ func (e *ErrSyntax) SrcContext() SrcCtx { return e.ctx } +// NewDependentObjectError creates a new ErrDependentObject. +func NewDependentObjectError(format string, args ...interface{}) error { + return &ErrDependentObject{ctx: MakeSrcCtx(1), msg: fmt.Sprintf(format, args...)} +} + +// ErrDependentObject represents a dependent object error. +type ErrDependentObject struct { + ctx SrcCtx + msg string +} + +func (e *ErrDependentObject) Error() string { + return e.msg +} + +// Code implements the ErrorWithPGCode interface. +func (*ErrDependentObject) Code() string { + return pgerror.CodeDependentObjectsStillExistError +} + +// SrcContext implements the ErrorWithPGCode interface. +func (e *ErrDependentObject) SrcContext() SrcCtx { + return e.ctx +} + // IsIntegrityConstraintError returns true if the error is some kind of SQL // constraint violation. func IsIntegrityConstraintError(err error) bool { diff --git a/pkg/sql/table.go b/pkg/sql/table.go index 6ef2c8dd4081..bb16da287f5e 100644 --- a/pkg/sql/table.go +++ b/pkg/sql/table.go @@ -518,16 +518,6 @@ func (p *planner) databaseFromSearchPath(tn *parser.TableName) (string, error) { return p.session.Database, nil } -// getQualifiedTableNameFromID returns the database-qualified name of the table -// or view with the provided ID. -func (p *planner) getQualifiedTableNameFromID(id sqlbase.ID) (string, error) { - desc, err := sqlbase.GetTableDescFromID(p.txn, id) - if err != nil { - return "", err - } - return p.getQualifiedTableName(desc) -} - // getQualifiedTableName returns the database-qualified name of the table // or view represented by the provided descriptor. func (p *planner) getQualifiedTableName(desc *sqlbase.TableDescriptor) (string, error) { diff --git a/pkg/sql/testdata/alter_table b/pkg/sql/testdata/alter_table index de4841378eaf..b2e32163a862 100644 --- a/pkg/sql/testdata/alter_table +++ b/pkg/sql/testdata/alter_table @@ -283,13 +283,13 @@ ALTER TABLE t ADD COLUMN e INT statement ok CREATE VIEW v AS SELECT x, y FROM t WHERE e > 5 -statement error cannot drop column "x" because it is depended on by view "test.v" +statement error cannot drop column "x" because view "v" depends on it ALTER TABLE t DROP COLUMN x -statement error cannot drop column "y" because it is depended on by view "test.v" +statement error cannot drop column "y" because view "v" depends on it ALTER TABLE t DROP COLUMN y -statement error cannot drop column "e" because it is depended on by view "test.v" +statement error cannot drop column "e" because view "v" depends on it ALTER TABLE t DROP COLUMN e statement ok @@ -307,7 +307,7 @@ CREATE VIEW v AS SELECT x, y FROM t WHERE e > 5 statement ok ALTER TABLE t DROP COLUMN IF EXISTS q -statement error cannot drop column "e" because it is depended on by view "test.v" +statement error cannot drop column "e" because view "v" depends on it ALTER TABLE t DROP COLUMN IF EXISTS e statement ok diff --git a/pkg/sql/testdata/drop_index b/pkg/sql/testdata/drop_index index 998d14440734..12f3d6c6bef0 100644 --- a/pkg/sql/testdata/drop_index +++ b/pkg/sql/testdata/drop_index @@ -113,7 +113,7 @@ users bar false 1 title ASC false statement ok CREATE VIEW v AS SELECT name FROM users@{FORCE_INDEX=foo} -statement error cannot drop index "foo" because it is depended on by view "test.v" +statement error cannot drop index "foo" because view "v" depends on it DROP INDEX users@foo statement ok diff --git a/pkg/sql/testdata/drop_view b/pkg/sql/testdata/drop_view index 1e764917fc66..04857429965b 100644 --- a/pkg/sql/testdata/drop_view +++ b/pkg/sql/testdata/drop_view @@ -17,13 +17,13 @@ a b c -statement error cannot drop table "a" because it is depended on by view "test.b" +statement error cannot drop table "a" because view "b" depends on it DROP TABLE a statement error pgcode 42809 "b" is not a table DROP TABLE b -statement error cannot drop view "b" because it is depended on by view "test.c" +statement error cannot drop view "b" because view "c" depends on it DROP VIEW b statement ok @@ -32,7 +32,7 @@ CREATE VIEW d AS SELECT k,v FROM a statement ok CREATE VIEW diamond AS SELECT COUNT(*) FROM b AS b JOIN d AS d ON b.k = d.k -statement error cannot drop view "d" because it is depended on by view "test.diamond" +statement error cannot drop view "d" because view "diamond" depends on it DROP VIEW d statement ok @@ -52,7 +52,7 @@ user testuser statement error user testuser does not have DROP privilege on view diamond DROP VIEW diamond -statement error cannot drop view "d" because it is depended on by view "test.diamond" +statement error cannot drop view "d" because view "diamond" depends on it DROP VIEW d user root @@ -103,10 +103,10 @@ diamond testuser1 testuser2 -statement error cannot drop view "testuser1" because it is depended on by view "test.testuser2" +statement error cannot drop view "testuser1" because view "testuser2" depends on it DROP VIEW testuser1 -statement error cannot drop view "testuser1" because it is depended on by view "test.testuser2" +statement error cannot drop view "testuser1" because view "testuser2" depends on it DROP VIEW testuser1 RESTRICT statement ok @@ -158,7 +158,7 @@ CREATE VIEW x AS VALUES (1, 2), (3, 4) statement ok CREATE VIEW y AS SELECT column1, column2 FROM x -statement error cannot drop view "x" because it is depended on by view "test.y" +statement error cannot drop view "x" because view "y" depends on it DROP VIEW x statement ok @@ -170,7 +170,7 @@ CREATE VIEW x AS VALUES (1, 2), (3, 4) statement ok CREATE VIEW y AS SELECT column1, column2 FROM x -statement error cannot drop view "x" because it is depended on by view "test.y" +statement error cannot drop view "x" because view "y" depends on it DROP VIEW x statement ok diff --git a/pkg/sql/testdata/rename_column b/pkg/sql/testdata/rename_column index 53bcbf768f96..8cc23c0db376 100644 --- a/pkg/sql/testdata/rename_column +++ b/pkg/sql/testdata/rename_column @@ -82,10 +82,10 @@ users bar true 2 username ASC false statement ok CREATE VIEW v1 AS SELECT id FROM users WHERE username = 'tom' -statement error cannot rename column "id" because it is depended on by view "test.v1" +statement error cannot rename column "id" because view "v1" depends on it ALTER TABLE users RENAME COLUMN id TO uid -statement error cannot rename column "username" because it is depended on by view "test.v1" +statement error cannot rename column "username" because view "v1" depends on it ALTER TABLE users RENAME COLUMN username TO name statement ok @@ -97,7 +97,7 @@ CREATE VIEW v2 AS SELECT id from users statement ok DROP VIEW v1 -statement error cannot rename column "id" because it is depended on by view "test.v2" +statement error cannot rename column "id" because view "v2" depends on it ALTER TABLE users RENAME COLUMN id TO uid statement ok diff --git a/pkg/sql/testdata/rename_database b/pkg/sql/testdata/rename_database index fd8c26164fff..7258d477a67d 100644 --- a/pkg/sql/testdata/rename_database +++ b/pkg/sql/testdata/rename_database @@ -108,7 +108,7 @@ SHOW TABLES FROM u ---- kv -statement error cannot rename database because table "kv" is depended on by view "t.v" +statement error cannot rename database because view "t.v" depends on table "kv" ALTER DATABASE u RENAME TO v statement ok @@ -120,5 +120,5 @@ ALTER DATABASE u RENAME TO v statement ok CREATE VIEW v.v AS SELECT k,v FROM v.kv -statement error cannot rename database because table "kv" is depended on by view "v.v" +statement error cannot rename database because view "v" depends on table "kv" ALTER DATABASE v RENAME TO u diff --git a/pkg/sql/testdata/rename_index b/pkg/sql/testdata/rename_index index 26d7c765262a..1a252879101f 100644 --- a/pkg/sql/testdata/rename_index +++ b/pkg/sql/testdata/rename_index @@ -81,7 +81,7 @@ SELECT * FROM users statement ok CREATE VIEW v AS SELECT name FROM users@{FORCE_INDEX=ufo} -statement error cannot rename index "ufo" because it is depended on by view "test.v" +statement error cannot rename index "ufo" because view "v" depends on it ALTER INDEX users@ufo RENAME TO foo statement ok diff --git a/pkg/sql/testdata/rename_table b/pkg/sql/testdata/rename_table index cb6e03defcb0..c3bad57fcb68 100644 --- a/pkg/sql/testdata/rename_table +++ b/pkg/sql/testdata/rename_table @@ -154,5 +154,5 @@ ALTER TABLE test2.v1 RENAME TO test2.v2 statement error pgcode 42809 "test2.v1" is not a table ALTER TABLE test2.v1 RENAME TO test2.v1 -statement error cannot rename table "test2.t2" because it is depended on by view "test2.v1" +statement error cannot rename table "test2.t2" because view "v1" depends on it ALTER TABLE test2.t2 RENAME TO test2.t3 diff --git a/pkg/sql/testdata/rename_view b/pkg/sql/testdata/rename_view index 12051c118152..daff7ad6cf61 100644 --- a/pkg/sql/testdata/rename_view +++ b/pkg/sql/testdata/rename_view @@ -166,14 +166,14 @@ SELECT * FROM test2.v2 statement ok CREATE VIEW v3 AS SELECT COUNT(*) FROM test2.v AS v JOIN test2.v2 AS v2 ON v.k > v2.c1 -statement error cannot rename view "test2.v" because it is depended on by view "test.v3" +statement error cannot rename view "test2.v" because view "test.v3" depends on it ALTER VIEW test2.v RENAME TO test2.v3 -statement error cannot rename view "test2.v2" because it is depended on by view "test.v3" +statement error cannot rename view "test2.v2" because view "test.v3" depends on it ALTER VIEW test2.v2 RENAME TO v4 statement ok ALTER VIEW v3 RENAME TO v4 -statement error cannot rename view "test2.v2" because it is depended on by view "test.v4" +statement error cannot rename view "test2.v2" because view "test.v4" depends on it ALTER VIEW test2.v2 RENAME TO v5 diff --git a/pkg/sql/testdata/views b/pkg/sql/testdata/views index 10fd9fc148d5..cba85c5033a5 100644 --- a/pkg/sql/testdata/views +++ b/pkg/sql/testdata/views @@ -214,10 +214,10 @@ DROP TABLE v1; statement error pgcode 42809 "t" is not a view DROP VIEW t; -statement error cannot drop view "v1" because it is depended on by view "test.v6" +statement error cannot drop view "v1" because view "v6" depends on it DROP VIEW v1; -statement error cannot drop view "v2" because it is depended on by view "test2.v1" +statement error cannot drop view "v2" because view "test2.v1" depends on it DROP VIEW v2; statement ok