Skip to content

Commit a457ff6

Browse files
rafissrafiss
committed
scbuild: implement RENAME CONSTRAINT in declarative schema changer
This patch implements full support for RENAME CONSTRAINT in the declarative schema changer. The implementation handles two types of constraints: 1. Index-backed constraints (PRIMARY KEY, UNIQUE with index): These rename by dropping the old IndexName element and adding a new one with the updated name. 2. Non-index constraints (CHECK, FK, UNIQUE without index): These rename by dropping the old ConstraintWithoutIndexName element and adding a new one with the updated name. The implementation includes proper validation: - Checks for constraint existence - Validates the constraint is not being added/dropped concurrently - Prevents name conflicts with existing constraints and indexes - Ensures views don't depend on index-backed constraints being renamed New dependency rules ensure the old name element reaches ABSENT before the new name element becomes PUBLIC, preventing conflicts and ensuring correct rollback behavior. Resolves: #148341 Epic: CRDB-31465 Release note: None
1 parent 2c3b545 commit a457ff6

17 files changed

+833
-7
lines changed

‎pkg/ccl/schemachangerccl/sctestbackupccl/backup_base_generated_test.go‎

Lines changed: 28 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/sql/schemachanger/scbuild/builder_state.go‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1402,6 +1402,8 @@ func (b *builderState) ResolveConstraint(
14021402
rel := b.descCache[relationID].desc.(catalog.TableDescriptor)
14031403
elts := b.QueryByID(rel.GetID())
14041404
var constraintID catid.ConstraintID
1405+
var indexID catid.IndexID
1406+
14051407
scpb.ForEachConstraintWithoutIndexName(elts,
14061408
func(status scpb.Status, _ scpb.TargetStatus, e *scpb.ConstraintWithoutIndexName) {
14071409
if tree.Name(e.Name) == constraintName {
@@ -1411,7 +1413,6 @@ func (b *builderState) ResolveConstraint(
14111413
)
14121414

14131415
if constraintID == 0 {
1414-
var indexID catid.IndexID
14151416
scpb.ForEachIndexName(elts, func(_ scpb.Status, _ scpb.TargetStatus, e *scpb.IndexName) {
14161417
if tree.Name(e.Name) == constraintName {
14171418
indexID = e.IndexID
@@ -1443,8 +1444,14 @@ func (b *builderState) ResolveConstraint(
14431444
}
14441445

14451446
return elts.Filter(func(_ scpb.Status, _ scpb.TargetStatus, e scpb.Element) bool {
1446-
idI, _ := screl.Schema.GetAttribute(screl.ConstraintID, e)
1447-
return idI != nil && idI.(catid.ConstraintID) == constraintID
1447+
constraintIDAttr, _ := screl.Schema.GetAttribute(screl.ConstraintID, e)
1448+
constraintIDMatches := constraintIDAttr != nil && constraintIDAttr.(catid.ConstraintID) == constraintID
1449+
1450+
// For index-backed constraints, we also want to include the elements that
1451+
// pertain to that index.
1452+
indexIDAttr, _ := screl.Schema.GetAttribute(screl.IndexID, e)
1453+
indexIDMatches := indexIDAttr != nil && indexID != 0 && indexIDAttr.(catid.IndexID) == indexID
1454+
return constraintIDMatches || indexIDMatches
14481455
})
14491456
}
14501457

‎pkg/sql/schemachanger/scbuild/internal/scbuildstmt/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ go_library(
1717
"alter_table_drop_column.go",
1818
"alter_table_drop_constraint.go",
1919
"alter_table_rename_column.go",
20+
"alter_table_rename_constraint.go",
2021
"alter_table_set_rls_mode.go",
2122
"alter_table_validate_constraint.go",
2223
"comment_on.go",

‎pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ var supportedAlterTableStatements = map[reflect.Type]supportedAlterTableCommand{
4444
reflect.TypeOf((*tree.AlterTableSetOnUpdate)(nil)): {fn: alterTableSetOnUpdate, on: true, checks: isV254Active},
4545
reflect.TypeOf((*tree.AlterTableRenameColumn)(nil)): {fn: alterTableRenameColumn, on: true, checks: isV254Active},
4646
reflect.TypeOf((*tree.AlterTableDropStored)(nil)): {fn: alterTableDropStored, on: true, checks: isV261Active},
47+
reflect.TypeOf((*tree.AlterTableRenameConstraint)(nil)): {fn: alterTableRenameConstraint, on: true, checks: isV261Active},
4748
}
4849

4950
func init() {

‎pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_rename_constraint.go‎

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
// Copyright 2025 The Cockroach Authors.
2+
//
3+
// Use of this software is governed by the CockroachDB Software License
4+
// included in the /LICENSE file.
5+
6+
package scbuildstmt
7+
8+
import (
9+
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
10+
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
11+
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
12+
"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb"
13+
"github.com/cockroachdb/cockroach/pkg/sql/sem/catid"
14+
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
15+
"github.com/cockroachdb/cockroach/pkg/sql/sqlerrors"
16+
)
17+
18+
func alterTableRenameConstraint(
19+
b BuildCtx,
20+
tn *tree.TableName,
21+
tbl *scpb.Table,
22+
n *tree.AlterTable,
23+
t *tree.AlterTableRenameConstraint,
24+
) {
25+
// Resolve the constraint by current name.
26+
constraintElems := b.ResolveConstraint(tbl.TableID, t.Constraint, ResolveParams{
27+
RequiredPrivilege: privilege.CREATE,
28+
})
29+
30+
// Validate constraint exists.
31+
if constraintElems == nil {
32+
panic(sqlerrors.NewUndefinedConstraintError(tree.ErrString(&t.Constraint), tn.Table()))
33+
}
34+
35+
// Short circuit if the name is unchanged.
36+
if t.Constraint == t.NewName {
37+
return
38+
}
39+
40+
// Handle both index-backed and non-index constraints.
41+
indexName := constraintElems.FilterIndexName().NotToAbsent().MustGetZeroOrOneElement()
42+
constraintWithoutIndexName := constraintElems.FilterConstraintWithoutIndexName().NotToAbsent().MustGetZeroOrOneElement()
43+
44+
if indexName == nil && constraintWithoutIndexName == nil {
45+
panic(pgerror.Newf(pgcode.UndefinedObject, "constraint %q does not exist", t.Constraint))
46+
}
47+
48+
// Check for name conflicts across all constraint types.
49+
// This must check both ConstraintWithoutIndexName (CHECK, FK, UNIQUE without
50+
// index) and IndexName (UNIQUE with index, PRIMARY KEY) to prevent a CHECK
51+
// constraint from being renamed to match a UNIQUE constraint name.
52+
checkConstraintNameConflicts(b, tbl.TableID, t.NewName)
53+
54+
// For index-backed constraints, check for dependent views.
55+
if indexName != nil {
56+
checkForDependentViews(b, tbl.TableID, indexName.IndexID, t.Constraint)
57+
}
58+
59+
// Rename based on constraint type.
60+
if indexName != nil {
61+
// Index-backed constraint (PRIMARY KEY, UNIQUE with index).
62+
b.Drop(indexName)
63+
b.Add(&scpb.IndexName{
64+
TableID: tbl.TableID,
65+
IndexID: indexName.IndexID,
66+
Name: string(t.NewName),
67+
})
68+
} else {
69+
// Non-index constraint (CHECK, FK, UNIQUE without index).
70+
b.Drop(constraintWithoutIndexName)
71+
b.Add(&scpb.ConstraintWithoutIndexName{
72+
TableID: tbl.TableID,
73+
ConstraintID: constraintWithoutIndexName.ConstraintID,
74+
Name: string(t.NewName),
75+
})
76+
}
77+
}
78+
79+
// checkConstraintNameConflicts verifies that the new constraint name does
80+
// not conflict with any existing constraint in the table, across all constraint
81+
// types. This checks both ConstraintWithoutIndexName elements (CHECK, FK, UNIQUE
82+
// without index) and IndexName elements (UNIQUE with index, PRIMARY KEY).
83+
// This comprehensive check is necessary because constraint names must be unique
84+
// across all types - a CHECK constraint cannot have the same name as a UNIQUE
85+
// constraint.
86+
func checkConstraintNameConflicts(b BuildCtx, tableID catid.DescID, newName tree.Name) {
87+
// Check ConstraintWithoutIndexName elements (CHECK, FK, UNIQUE without index).
88+
tableElems := b.QueryByID(tableID)
89+
conflictingConstraints := tableElems.FilterConstraintWithoutIndexName().Filter(func(
90+
_ scpb.Status, target scpb.TargetStatus, e *scpb.ConstraintWithoutIndexName,
91+
) bool {
92+
return tree.Name(e.Name) == newName && target == scpb.ToPublic
93+
})
94+
95+
if !conflictingConstraints.IsEmpty() {
96+
panic(pgerror.Newf(pgcode.DuplicateObject, "duplicate constraint name: %q", newName))
97+
}
98+
99+
// Check IndexName elements. These could be either index-backed constraints
100+
// (UNIQUE with index, PRIMARY KEY) or regular indexes.
101+
conflictingIndexNames := tableElems.FilterIndexName().Filter(func(
102+
_ scpb.Status, target scpb.TargetStatus, e *scpb.IndexName,
103+
) bool {
104+
return e.Name == string(newName) && target == scpb.ToPublic
105+
})
106+
107+
conflictingIndexNames.ForEach(func(_ scpb.Status, _ scpb.TargetStatus, indexNameElem *scpb.IndexName) {
108+
// Check if this index backs a constraint by looking for corresponding
109+
// index elements with ConstraintID set.
110+
secondaryIndex := tableElems.FilterSecondaryIndex().Filter(func(
111+
_ scpb.Status, _ scpb.TargetStatus, idx *scpb.SecondaryIndex,
112+
) bool {
113+
return idx.IndexID == indexNameElem.IndexID && idx.ConstraintID != 0
114+
})
115+
116+
primaryIndex := tableElems.FilterPrimaryIndex().Filter(func(
117+
_ scpb.Status, _ scpb.TargetStatus, idx *scpb.PrimaryIndex,
118+
) bool {
119+
return idx.IndexID == indexNameElem.IndexID
120+
})
121+
122+
isConstraintIndex := !secondaryIndex.IsEmpty() || !primaryIndex.IsEmpty()
123+
124+
if isConstraintIndex {
125+
panic(pgerror.Newf(pgcode.DuplicateObject, "duplicate constraint name: %q", newName))
126+
} else {
127+
panic(pgerror.Newf(pgcode.DuplicateRelation, "relation %v already exists", newName))
128+
}
129+
})
130+
}
131+
132+
// checkForDependentViews ensures that views don't depend on the index being
133+
// renamed. This follows the same logic as the legacy schema changer.
134+
func checkForDependentViews(
135+
b BuildCtx, tableID catid.DescID, indexID catid.IndexID, constraintName tree.Name,
136+
) {
137+
// Check for views that depend on this index.
138+
scpb.ForEachView(b.BackReferences(tableID), func(
139+
current scpb.Status, target scpb.TargetStatus, e *scpb.View,
140+
) {
141+
// Check if the view depends on this specific index.
142+
for _, forwardRef := range e.ForwardReferences {
143+
if forwardRef.IndexID != indexID {
144+
continue
145+
}
146+
147+
// This view depends on the index-backed constraint being renamed.
148+
viewName := b.QueryByID(e.ViewID).FilterNamespace().MustGetOneElement()
149+
tableName := b.QueryByID(tableID).FilterNamespace().MustGetOneElement()
150+
// Check if view is in the same database and schema.
151+
if viewName.DatabaseID != tableName.DatabaseID || viewName.SchemaID != tableName.SchemaID {
152+
panic(sqlerrors.NewDependentBlocksOpError("rename", "constraint",
153+
tree.ErrString(&constraintName), "view", qualifiedName(b, e.ViewID)))
154+
}
155+
panic(sqlerrors.NewDependentBlocksOpError("rename", "constraint",
156+
tree.ErrString(&constraintName), "view", viewName.Name))
157+
}
158+
})
159+
}

‎pkg/sql/schemachanger/scbuild/testdata/alter_table_rename_constraint‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
setup
2+
CREATE TABLE t (i INT PRIMARY KEY, j INT, CONSTRAINT cc CHECK (i > 0));
3+
----
4+
5+
build
6+
ALTER TABLE t RENAME CONSTRAINT cc TO cc_renamed;
7+
----
8+
- [[IndexData:{DescID: 104, IndexID: 1}, PUBLIC], PUBLIC]
9+
{indexId: 1, tableId: 104}
10+
- [[ConstraintWithoutIndexName:{DescID: 104, Name: cc, ConstraintID: 2}, ABSENT], PUBLIC]
11+
{constraintId: 2, name: cc, tableId: 104}
12+
- [[TableData:{DescID: 104, ReferencedDescID: 100}, PUBLIC], PUBLIC]
13+
{databaseId: 100, tableId: 104}
14+
- [[ConstraintWithoutIndexName:{DescID: 104, Name: cc_renamed, ConstraintID: 2}, PUBLIC], ABSENT]
15+
{constraintId: 2, name: cc_renamed, tableId: 104}
16+
17+
setup
18+
CREATE TABLE t2 (i INT PRIMARY KEY, CONSTRAINT u UNIQUE (i));
19+
----
20+
21+
build
22+
ALTER TABLE t2 RENAME CONSTRAINT u TO u_renamed;
23+
----
24+
- [[IndexData:{DescID: 105, IndexID: 1}, PUBLIC], PUBLIC]
25+
{indexId: 1, tableId: 105}
26+
- [[IndexName:{DescID: 105, Name: u, IndexID: 2}, ABSENT], PUBLIC]
27+
{indexId: 2, name: u, tableId: 105}
28+
- [[IndexData:{DescID: 105, IndexID: 2}, PUBLIC], PUBLIC]
29+
{indexId: 2, tableId: 105}
30+
- [[TableData:{DescID: 105, ReferencedDescID: 100}, PUBLIC], PUBLIC]
31+
{databaseId: 100, tableId: 105}
32+
- [[IndexName:{DescID: 105, Name: u_renamed, IndexID: 2}, PUBLIC], ABSENT]
33+
{indexId: 2, name: u_renamed, tableId: 105}
34+
35+
setup
36+
CREATE TABLE t3 (i INT PRIMARY KEY);
37+
CREATE TABLE t4 (i INT REFERENCES t3(i));
38+
----
39+
40+
build
41+
ALTER TABLE t4 RENAME CONSTRAINT t4_i_fkey TO fk_renamed;
42+
----
43+
- [[IndexData:{DescID: 107, IndexID: 1}, PUBLIC], PUBLIC]
44+
{indexId: 1, tableId: 107}
45+
- [[ConstraintWithoutIndexName:{DescID: 107, Name: t4_i_fkey, ConstraintID: 2}, ABSENT], PUBLIC]
46+
{constraintId: 2, name: t4_i_fkey, tableId: 107}
47+
- [[TableData:{DescID: 107, ReferencedDescID: 100}, PUBLIC], PUBLIC]
48+
{databaseId: 100, tableId: 107}
49+
- [[ConstraintWithoutIndexName:{DescID: 107, Name: fk_renamed, ConstraintID: 2}, PUBLIC], ABSENT]
50+
{constraintId: 2, name: fk_renamed, tableId: 107}
51+
52+
setup
53+
CREATE TABLE t5 (i INT PRIMARY KEY, j INT, CONSTRAINT con1 CHECK (i > 0), CONSTRAINT con2 CHECK (j > 0));
54+
----
55+
56+
build
57+
ALTER TABLE t5 RENAME CONSTRAINT con1 TO con1_old, RENAME CONSTRAINT con2 TO con1;
58+
----
59+
- [[IndexData:{DescID: 108, IndexID: 1}, PUBLIC], PUBLIC]
60+
{indexId: 1, tableId: 108}
61+
- [[ConstraintWithoutIndexName:{DescID: 108, Name: con1, ConstraintID: 2}, ABSENT], PUBLIC]
62+
{constraintId: 2, name: con1, tableId: 108}
63+
- [[ConstraintWithoutIndexName:{DescID: 108, Name: con2, ConstraintID: 3}, ABSENT], PUBLIC]
64+
{constraintId: 3, name: con2, tableId: 108}
65+
- [[TableData:{DescID: 108, ReferencedDescID: 100}, PUBLIC], PUBLIC]
66+
{databaseId: 100, tableId: 108}
67+
- [[ConstraintWithoutIndexName:{DescID: 108, Name: con1_old, ConstraintID: 2}, PUBLIC], ABSENT]
68+
{constraintId: 2, name: con1_old, tableId: 108}
69+
- [[ConstraintWithoutIndexName:{DescID: 108, Name: con1, ConstraintID: 3}, PUBLIC], ABSENT]
70+
{constraintId: 3, name: con1, tableId: 108}

‎pkg/sql/schemachanger/scbuild/testdata/unimplemented_alter_table‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,6 @@ CREATE TABLE defaultdb.foo (
1212
);
1313
----
1414

15-
unimplemented
16-
ALTER TABLE defaultdb.foo RENAME CONSTRAINT foobar TO baz
17-
----
18-
1915
unimplemented
2016
ALTER TABLE defaultdb.foo EXPERIMENTAL_AUDIT SET READ WRITE
2117
----

‎pkg/sql/schemachanger/scplan/internal/rules/current/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ go_library(
2323
"dep_drop_trigger.go",
2424
"dep_garbage_collection.go",
2525
"dep_rename_column.go",
26+
"dep_rename_constraint.go",
2627
"dep_rename_table.go",
2728
"dep_schema_locked.go",
2829
"dep_swap_index.go",

0 commit comments

Comments
 (0)