diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go index 10cb850f01b5..69b4b24360ea 100644 --- a/pkg/sql/alter_table.go +++ b/pkg/sql/alter_table.go @@ -1589,6 +1589,7 @@ func dropColumnImpl( // You can't drop a column depended on by a view unless CASCADE was // specified. + var depsToDrop catalog.DescriptorIDSet for _, ref := range tableDesc.DependedOnBy { found := false for _, colID := range ref.ColumnIDs { @@ -1606,32 +1607,14 @@ func dropColumnImpl( if err != nil { return nil, err } - depDesc, err := params.p.getDescForCascade( - params.ctx, "column", string(t.Column), tableDesc.ParentID, ref.ID, t.DropBehavior, - ) - if err != nil { - return nil, err - } - switch t := depDesc.(type) { - case *tabledesc.Mutable: - jobDesc := fmt.Sprintf("removing view %q dependent on column %q which is being dropped", - t.Name, colToDrop.ColName()) - cascadedViews, err := params.p.removeDependentView(params.ctx, tableDesc, t, jobDesc) - if err != nil { - return nil, err - } - qualifiedView, err := params.p.getQualifiedTableName(params.ctx, t) - if err != nil { - return nil, err - } + depsToDrop.Add(ref.ID) + } - droppedViews = append(droppedViews, cascadedViews...) - droppedViews = append(droppedViews, qualifiedView.FQString()) - case *funcdesc.Mutable: - if err := params.p.removeDependentFunction(params.ctx, tableDesc, t); err != nil { - return nil, err - } - } + droppedViews, err = params.p.removeDependents( + params.ctx, tableDesc, depsToDrop, "column", colToDrop.GetName(), t.DropBehavior, + ) + if err != nil { + return nil, err } // We cannot remove this column if there are computed columns or a TTL diff --git a/pkg/sql/drop_index.go b/pkg/sql/drop_index.go index 3f7db4ca162e..762ece236627 100644 --- a/pkg/sql/drop_index.go +++ b/pkg/sql/drop_index.go @@ -424,6 +424,7 @@ func (p *planner) dropIndexByName( } var droppedViews []string + var depsToDrop catalog.DescriptorIDSet for _, tableRef := range tableDesc.DependedOnBy { if tableRef.IndexID == idx.GetID() { // Ensure that we have DROP privilege on all dependent views @@ -432,36 +433,17 @@ func (p *planner) dropIndexByName( if err != nil { return err } - depDesc, err := p.getDescForCascade( - ctx, "index", idx.GetName(), tableDesc.ParentID, tableRef.ID, behavior, - ) - if err != nil { - return err - } - switch t := depDesc.(type) { - case *tabledesc.Mutable: - viewJobDesc := fmt.Sprintf("removing view %q dependent on index %q which is being dropped", - t.Name, idx.GetName()) - cascadedViews, err := p.removeDependentView(ctx, tableDesc, t, viewJobDesc) - if err != nil { - return err - } - - qualifiedView, err := p.getQualifiedTableName(ctx, t) - if err != nil { - return err - } - - droppedViews = append(droppedViews, qualifiedView.FQString()) - droppedViews = append(droppedViews, cascadedViews...) - case *funcdesc.Mutable: - if err := p.removeDependentFunction(ctx, tableDesc, t); err != nil { - return err - } - } + depsToDrop.Add(tableRef.ID) } } + droppedViews, err = p.removeDependents( + ctx, tableDesc, depsToDrop, "index", idx.GetName(), behavior, + ) + if err != nil { + return err + } + // Overwriting tableDesc.Index may mess up with the idx object we collected above. Make a copy. idxCopy := *idx.IndexDesc() idxDesc := &idxCopy @@ -533,3 +515,45 @@ func (p *planner) dropIndexByName( CascadeDroppedViews: droppedViews, }) } + +func (p *planner) removeDependents( + ctx context.Context, + tableDesc *tabledesc.Mutable, + depsToDrop catalog.DescriptorIDSet, + typeName string, + objName string, + dropBehavior tree.DropBehavior, +) (droppedViews []string, err error) { + for _, descId := range depsToDrop.Ordered() { + depDesc, err := p.getDescForCascade( + ctx, typeName, objName, tableDesc.ParentID, descId, dropBehavior, + ) + if err != nil { + return nil, err + } + if depDesc.Dropped() { + continue + } + switch t := depDesc.(type) { + case *tabledesc.Mutable: + jobDesc := fmt.Sprintf("removing view %q dependent on %s %q which is being dropped", + t.Name, typeName, objName) + cascadedViews, err := p.removeDependentView(ctx, tableDesc, t, jobDesc) + if err != nil { + return nil, err + } + qualifiedView, err := p.getQualifiedTableName(ctx, t) + if err != nil { + return nil, err + } + + droppedViews = append(droppedViews, cascadedViews...) + droppedViews = append(droppedViews, qualifiedView.FQString()) + case *funcdesc.Mutable: + if err := p.removeDependentFunction(ctx, tableDesc, t); err != nil { + return nil, err + } + } + } + return nil, nil +} diff --git a/pkg/sql/drop_table.go b/pkg/sql/drop_table.go index 3f93a31d6000..b84912dc5b97 100644 --- a/pkg/sql/drop_table.go +++ b/pkg/sql/drop_table.go @@ -647,7 +647,7 @@ func removeFKBackReferenceFromTable( func removeMatchingReferences( refs []descpb.TableDescriptor_Reference, id descpb.ID, ) []descpb.TableDescriptor_Reference { - updatedRefs := refs[:0] + var updatedRefs []descpb.TableDescriptor_Reference for _, ref := range refs { if ref.ID != id { updatedRefs = append(updatedRefs, ref) diff --git a/pkg/sql/logictest/testdata/logic_test/alter_table b/pkg/sql/logictest/testdata/logic_test/alter_table index 154ba08557c7..2396b152cd12 100644 --- a/pkg/sql/logictest/testdata/logic_test/alter_table +++ b/pkg/sql/logictest/testdata/logic_test/alter_table @@ -2940,3 +2940,74 @@ ALTER TABLE t_96648 DROP CONSTRAINT check_i, DROP CONSTRAINT check_i1; statement ok DROP TABLE t_96648 + +subtest regression_97546 + +statement ok +CREATE TABLE t_twocol ( + id INT PRIMARY KEY, + a INT, + b INT, + FAMILY fam_0 (id, a, b) +); +CREATE FUNCTION f1() RETURNS t_twocol AS 'SELECT * FROM t_twocol' LANGUAGE SQL; +CREATE FUNCTION f2() RETURNS t_twocol AS 'SELECT * FROM t_twocol' LANGUAGE SQL; +CREATE FUNCTION f3() RETURNS INT AS 'SELECT a FROM t_twocol' LANGUAGE SQL; + +statement ok +ALTER TABLE t_twocol DROP COLUMN b CASCADE; + +statement error pq: unknown function: f1\(\): function undefined +SELECT f1(); + +statement error pq: unknown function: f2\(\): function undefined +SELECT f2(); + +query T +SELECT create_statement FROM [SHOW CREATE TABLE t_twocol] +---- +CREATE TABLE public.t_twocol ( + id INT8 NOT NULL, + a INT8 NULL, + CONSTRAINT t_twocol_pkey PRIMARY KEY (id ASC), + FAMILY fam_0 (id, a) +) + +statement ok +DROP TABLE t_twocol CASCADE; + +subtest regress_96368 + +statement ok +CREATE TABLE t_twocol ( + id INT PRIMARY KEY, + a INT, + b INT, + FAMILY fam_0 (id, a, b) +); +CREATE FUNCTION f_unqualified_twocol() RETURNS t_twocol AS +$$ + SELECT t_twocol.id, t_twocol.a, t_twocol.b FROM t_twocol; +$$ LANGUAGE SQL; +CREATE FUNCTION f_allcolsel_alias() RETURNS t_twocol AS +$$ + SELECT t1.id, t1.a, t1.b FROM t_twocol AS t1, t_twocol AS t2 WHERE t1.a = t2.a; +$$ LANGUAGE SQL; +CREATE FUNCTION f_tuplestar() RETURNS t_twocol AS +$$ + SELECT t_twocol.id, t_twocol.a, t_twocol.b FROM t_twocol; +$$ LANGUAGE SQL; + +# This statement should be ok but fails +statement ok +ALTER TABLE t_twocol DROP COLUMN b CASCADE; + +query T +SELECT create_statement FROM [SHOW CREATE TABLE t_twocol] +---- +CREATE TABLE public.t_twocol ( + id INT8 NOT NULL, + a INT8 NULL, + CONSTRAINT t_twocol_pkey PRIMARY KEY (id ASC), + FAMILY fam_0 (id, a) +)