diff --git a/pkg/sql/drop_database.go b/pkg/sql/drop_database.go index cb19c9d0f0b6..7c5e69519aa8 100644 --- a/pkg/sql/drop_database.go +++ b/pkg/sql/drop_database.go @@ -253,8 +253,9 @@ func (p *planner) accumulateAllObjectsToDelete( ctx context.Context, objects []toDelete, ) ([]*tabledesc.Mutable, map[descpb.ID]*tabledesc.Mutable, error) { implicitDeleteObjects := make(map[descpb.ID]*tabledesc.Mutable) + visited := make(map[descpb.ID]struct{}) for _, toDel := range objects { - err := p.accumulateCascadingViews(ctx, implicitDeleteObjects, toDel.desc) + err := p.accumulateCascadingViews(ctx, implicitDeleteObjects, visited, toDel.desc) if err != nil { return nil, nil, err } @@ -311,8 +312,12 @@ func (p *planner) accumulateOwnedSequences( // references, which means this list can't be constructed by simply scanning // the namespace table. func (p *planner) accumulateCascadingViews( - ctx context.Context, dependentObjects map[descpb.ID]*tabledesc.Mutable, desc *tabledesc.Mutable, + ctx context.Context, + dependentObjects map[descpb.ID]*tabledesc.Mutable, + visited map[descpb.ID]struct{}, + desc *tabledesc.Mutable, ) error { + visited[desc.ID] = struct{}{} for _, ref := range desc.DependedOnBy { desc, err := p.Descriptors().MutableByID(p.txn).Desc(ctx, ref.ID) if err != nil { @@ -327,8 +332,15 @@ func (p *planner) accumulateCascadingViews( if !dependentDesc.IsView() { continue } + + _, seen := visited[ref.ID] + if dependentObjects[ref.ID] == dependentDesc || seen { + // This view's dependencies are already added. + continue + } dependentObjects[ref.ID] = dependentDesc - if err := p.accumulateCascadingViews(ctx, dependentObjects, dependentDesc); err != nil { + + if err := p.accumulateCascadingViews(ctx, dependentObjects, visited, dependentDesc); err != nil { return err } } diff --git a/pkg/sql/drop_view.go b/pkg/sql/drop_view.go index 8146bd68aec6..1a3d2a897b94 100644 --- a/pkg/sql/drop_view.go +++ b/pkg/sql/drop_view.go @@ -21,6 +21,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc" "github.com/cockroachdb/cockroach/pkg/sql/privilege" "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scerrors" + "github.com/cockroachdb/cockroach/pkg/sql/sem/catid" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry" "github.com/cockroachdb/cockroach/pkg/util/log" @@ -170,6 +171,18 @@ func (p *planner) canRemoveDependentFromTable( ref descpb.TableDescriptor_Reference, behavior tree.DropBehavior, ) error { + if p.trackDependency == nil { + p.trackDependency = make(map[catid.DescID]bool) + } + if p.trackDependency[ref.ID] { + // This table's dependencies are already tracked. + return nil + } + p.trackDependency[ref.ID] = true + defer func() { + p.trackDependency[ref.ID] = false + }() + return p.canRemoveDependent(ctx, string(from.DescriptorType()), from.Name, from.ParentID, ref, behavior) } diff --git a/pkg/sql/logictest/testdata/logic_test/views b/pkg/sql/logictest/testdata/logic_test/views index e70879165ce8..50c7c83845bb 100644 --- a/pkg/sql/logictest/testdata/logic_test/views +++ b/pkg/sql/logictest/testdata/logic_test/views @@ -22,12 +22,12 @@ CREATE VIEW v1 AS SELECT a, b FROM t statement error pgcode 42P07 relation \"test.public.t\" already exists CREATE VIEW t AS SELECT a, b FROM t -# view statement ignored if other way around. statement ok -CREATE VIEW IF NOT EXISTS v1 AS SELECT b, a FROM v1 +CREATE VIEW IF NOT EXISTS v2 (x, y) AS SELECT a, b FROM t +# view statement ignored if other way around. statement ok -CREATE VIEW IF NOT EXISTS v2 (x, y) AS SELECT a, b FROM t +CREATE VIEW IF NOT EXISTS v2 AS SELECT b, a FROM v1 statement error pgcode 42601 CREATE VIEW specifies 1 column name, but data source has 2 columns CREATE VIEW v3 (x) AS SELECT a, b FROM t @@ -1813,3 +1813,37 @@ SELECT * FROM v; statement ok SET DATABASE = test; + +subtest circular_dependency + +statement ok +CREATE TABLE t (a INT PRIMARY KEY, b INT); + +statement ok +CREATE VIEW cd_v1 AS SELECT a, b FROM t; + +statement ok +CREATE VIEW cd_v2 AS SELECT a, b FROM cd_v1; + +# Note: Creating a circular dependency in views does not result in an error in +# postgres. In postgres, we only encounter errors during queries on the views. +statement error pgcode 42P17 pq: cyclic view dependency for relation test.public.cd_v1 +CREATE OR REPLACE VIEW cd_v1 AS SELECT a, b FROM cd_v2; + +statement ok +CREATE VIEW cd_v3 AS SELECT a, b FROM cd_v2; + +statement ok +SELECT * FROM cd_v3; + +statement error pgcode 42P17 pq: cyclic view dependency for relation test.public.cd_v1 +CREATE OR REPLACE VIEW cd_v1 AS SELECT a, b FROM cd_v3; + +statement ok +SELECT * FROM cd_v3; + +statement error pq: cannot drop relation "cd_v1" because view "cd_v2" depends on it +DROP VIEW cd_v1; + +statement ok +DROP VIEW cd_v1 CASCADE; diff --git a/pkg/sql/opt/optbuilder/builder.go b/pkg/sql/opt/optbuilder/builder.go index 207f60ab45af..483ffcf9c8b8 100644 --- a/pkg/sql/opt/optbuilder/builder.go +++ b/pkg/sql/opt/optbuilder/builder.go @@ -113,6 +113,10 @@ type Builder struct { // are referenced multiple times in the same query. views map[cat.View]*tree.Select + // sourceViews contains a map with all the views in the current data source + // chain. It is used to detect circular dependencies. + sourceViews map[string]struct{} + // subquery contains a pointer to the subquery which is currently being built // (if any). subquery *subquery diff --git a/pkg/sql/opt/optbuilder/create_view.go b/pkg/sql/opt/optbuilder/create_view.go index b5b508d4cbe9..7d217bc1cdae 100644 --- a/pkg/sql/opt/optbuilder/create_view.go +++ b/pkg/sql/opt/optbuilder/create_view.go @@ -36,12 +36,17 @@ func (b *Builder) buildCreateView(cv *tree.CreateView, inScope *scope) (outScope b.insideViewDef = true b.trackSchemaDeps = true b.qualifyDataSourceNamesInAST = true + if b.sourceViews == nil { + b.sourceViews = make(map[string]struct{}) + } + b.sourceViews[viewName.FQString()] = struct{}{} defer func() { b.insideViewDef = false b.trackSchemaDeps = false b.schemaDeps = nil b.schemaTypeDeps = intsets.Fast{} b.qualifyDataSourceNamesInAST = false + delete(b.sourceViews, viewName.FQString()) b.semaCtx.FunctionResolver = preFuncResolver switch recErr := recover().(type) { diff --git a/pkg/sql/opt/optbuilder/select.go b/pkg/sql/opt/optbuilder/select.go index 8af723363358..0325ce381ad0 100644 --- a/pkg/sql/opt/optbuilder/select.go +++ b/pkg/sql/opt/optbuilder/select.go @@ -280,6 +280,17 @@ func (b *Builder) buildDataSource( func (b *Builder) buildView( view cat.View, viewName *tree.TableName, locking lockingSpec, inScope *scope, ) (outScope *scope) { + if b.sourceViews == nil { + b.sourceViews = make(map[string]struct{}) + } + // Check whether there is a circular dependency between views. + if _, ok := b.sourceViews[viewName.FQString()]; ok { + panic(pgerror.Newf(pgcode.InvalidObjectDefinition, "cyclic view dependency for relation %s", viewName)) + } + b.sourceViews[viewName.FQString()] = struct{}{} + defer func() { + delete(b.sourceViews, viewName.FQString()) + }() // Cache the AST so that multiple references won't need to reparse. if b.views == nil { b.views = make(map[cat.View]*tree.Select) diff --git a/pkg/sql/planner.go b/pkg/sql/planner.go index a41eb6081a21..dbe81ea5cbf9 100644 --- a/pkg/sql/planner.go +++ b/pkg/sql/planner.go @@ -40,6 +40,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/parser" "github.com/cockroachdb/cockroach/pkg/sql/privilege" "github.com/cockroachdb/cockroach/pkg/sql/querycache" + "github.com/cockroachdb/cockroach/pkg/sql/sem/catid" "github.com/cockroachdb/cockroach/pkg/sql/sem/eval" "github.com/cockroachdb/cockroach/pkg/sql/sem/transform" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" @@ -262,6 +263,9 @@ type planner struct { // evalCatalogBuiltins is used as part of the eval.Context. evalCatalogBuiltins evalcatalog.Builtins + + // trackDependency is used to track circular dependencies when dropping views. + trackDependency map[catid.DescID]bool } // hasFlowForPausablePortal returns true if the planner is for re-executing a