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.

Sign up for GitHub

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

sql: create new slice when removing backreference items #97631

Merged
merged 1 commit into from
Feb 28, 2023
Merged

sql: create new slice when removing backreference items #97631

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 8 additions & 25 deletions pkg/sql/alter_table.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1615,6 +1615,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 {
Expand All @@ -1632,32 +1633,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
Expand Down
78 changes: 51 additions & 27 deletions pkg/sql/drop_index.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
}
71 changes: 71 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/alter_table
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3064,3 +3064,74 @@ t_96727_2 CREATE TABLE public.t_96727_2 (
CONSTRAINT t_96727_2_pkey PRIMARY KEY (i ASC),
FAMILY fam_0_i_j (i)
)

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)
)
9 changes: 9 additions & 0 deletions pkg/sql/schemachanger/scdecomp/decomp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catenumpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scerrors"
"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catid"
Expand Down Expand Up @@ -116,6 +117,14 @@ func (w *walkCtx) walkRoot() {
case catalog.TableDescriptor:
w.walkRelation(d)
case catalog.FunctionDescriptor:
if !w.clusterVersion.IsActive(clusterversion.V23_1) {
panic(
scerrors.NotImplementedErrorf(
nil, // n
"function relevant elements and rules are not supported until fully upgraded to 23.1",
),
)
}
w.walkFunction(d)
default:
panic(errors.AssertionFailedf("unexpected descriptor type %T: %+v",
Expand Down