Skip to content

Commit 68c8641

Browse files
committed
sql: add ALTER TABLE ... SET SCHEMA to the declerative schema changer
This change adds support for `ALTER TABLE ... SET SCHEMA` to the declarative schema changer. Before, this operation was ran by the legacy schema changer. Epic CRDB-31281 Fixes #155989 Release note (sql change): `ALTER TABLE ... SET SCHEMA` is supported by the declerative schema changer.
1 parent 1a2a0ab commit 68c8641

File tree

12 files changed

+355
-47
lines changed

12 files changed

+355
-47
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/drop_function_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -249,8 +249,9 @@ USE defaultdb;
249249
dscExpectedErr: `pq: cannot rename relation "defaultdb.public.t" because function "f" depends on it`,
250250
},
251251
{
252-
stmt: "ALTER TABLE t SET SCHEMA test_sc",
253-
expectedErr: `pq: cannot set schema on relation "t" because function "f" depends on it`,
252+
stmt: "ALTER TABLE t SET SCHEMA test_sc",
253+
expectedErr: `pq: cannot set schema on relation "t" because function "f" depends on it`,
254+
dscExpectedErr: `pq: cannot set schema on relation "defaultdb.public.t" because function "f" depends on it`,
254255
},
255256
{
256257
stmt: "ALTER TABLE t DROP COLUMN d",

pkg/sql/logictest/testdata/logic_test/alter_table

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4508,6 +4508,24 @@ RESET create_table_with_schema_locked
45084508

45094509
subtest end
45104510

4511+
subtest alter_table_set_schema
4512+
4513+
statement ok
4514+
CREATE TABLE non_public (c int);
4515+
CREATE SCHEMA sc1;
4516+
SELECT * FROM non_public;
4517+
ALTER TABLE non_public SET SCHEMA sc1;
4518+
4519+
statement error pgcode 42P01 relation "non_public" does not exist
4520+
SELECT * FROM non_public
4521+
4522+
statement ok
4523+
SELECT * from sc1.non_public;
4524+
DROP TABLE sc1.non_public;
4525+
DROP SCHEMA sc1;
4526+
4527+
subtest end
4528+
45114529
subtest alter_primary_key_using_dropped_column
45124530

45134531
statement ok

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ go_library(
2121
"alter_table_rename_column.go",
2222
"alter_table_rename_constraint.go",
2323
"alter_table_set_rls_mode.go",
24+
"alter_table_set_schema.go",
2425
"alter_table_validate_constraint.go",
2526
"comment_on.go",
2627
"configure_zone.go",
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
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+
)
16+
17+
// AlterTableSetSchema implements ALTER TABLE ... SET SCHEMA ... for the declarative schema changer.
18+
// It sets the schema for a table, view, or sequence.
19+
// Requires privileges: DROP on source table/view/sequence, CREATE on destination schema.
20+
func AlterTableSetSchema(b BuildCtx, n *tree.AlterTableSetSchema) {
21+
// Resolve any type of object
22+
elts := b.ResolveRelation(n.Name, ResolveParams{
23+
IsExistenceOptional: n.IfExists,
24+
RequiredPrivilege: privilege.DROP,
25+
})
26+
// IF EXISTS was specified, and the object doesn't exist, so this is a no-op.
27+
if elts == nil && n.IfExists {
28+
return
29+
}
30+
// Validate the object type matches what was requested.
31+
validateObjectType(elts, n.Name.ToTableName().ObjectName, n.IsSequence, n.IsView, n.IsMaterialized)
32+
33+
// get descId based on type to retrieve the namespace
34+
descID, element, isTemp := getAlterTableTargetElement(elts)
35+
// Ensure that table is not temporary
36+
if isTemp {
37+
panic(pgerror.Newf(pgcode.FeatureNotSupported,
38+
"cannot move objects into or out of temporary schemas"))
39+
}
40+
// Get the fully qualified object name.
41+
currName := getAlterTableQualifiedObjectName(n.Name, b)
42+
newName := currName
43+
newName.SchemaName = n.Schema
44+
// Check for name-based dependencies
45+
checkNameBasedDependencies(b, descID, element, currName, "set schema on")
46+
47+
// Get the schema ID directly from the namespace element
48+
currNamespace := mustRetrieveNamespaceElem(b, descID)
49+
currSchemaID := currNamespace.SchemaID
50+
newSchema := resolveSchemaByName(b, n.Schema, currNamespace.DatabaseID)
51+
newSchemaID := newSchema.SchemaID
52+
// Ensure that new schema is not temporary or virtual
53+
panicIfSchemaIsTemporaryOrVirtual(newSchema)
54+
// If new schema is the same as the curr schema, do a no-op
55+
if currSchemaID == newSchemaID {
56+
return
57+
}
58+
59+
// Increment telemetry counter
60+
b.IncrementSchemaChangeAlterCounter(tree.GetTableType(n.IsSequence, n.IsView, n.IsMaterialized), n.TelemetryName())
61+
// Check for name conflicts
62+
checkTableNameConflicts(b, currName, newName, currNamespace)
63+
64+
// drop the old namespace and add a new one
65+
newNamespace := *currNamespace
66+
newNamespace.SchemaID = newSchemaID
67+
b.Drop(currNamespace)
68+
b.Add(&newNamespace)
69+
70+
// drop old schema child and add new one
71+
currSchemaChild := b.QueryByID(descID).FilterSchemaChild().MustGetOneElement()
72+
newSchemaChild := scpb.SchemaChild{
73+
ChildObjectID: descID,
74+
SchemaID: newSchemaID,
75+
}
76+
b.Drop(currSchemaChild)
77+
b.Add(&newSchemaChild)
78+
79+
}
80+
81+
func resolveSchemaByName(b BuildCtx, schemaName tree.Name, databaseID catid.DescID) *scpb.Schema {
82+
dbElts := b.QueryByID(databaseID)
83+
dbNamespace := dbElts.FilterNamespace().MustGetOneElement()
84+
// Resolve the new schema to get its elements
85+
newSchemaPrefix := tree.ObjectNamePrefix{
86+
CatalogName: tree.Name(dbNamespace.Name),
87+
SchemaName: schemaName,
88+
ExplicitCatalog: true,
89+
ExplicitSchema: true,
90+
}
91+
newSchema := b.ResolveSchema(newSchemaPrefix, ResolveParams{
92+
RequiredPrivilege: privilege.CREATE,
93+
}).FilterSchema().MustGetOneElement()
94+
return newSchema
95+
}
96+
97+
func panicIfSchemaIsTemporaryOrVirtual(newSchema *scpb.Schema) {
98+
if newSchema.IsTemporary {
99+
panic(pgerror.Newf(pgcode.FeatureNotSupported,
100+
"cannot move objects into or out of temporary schemas"))
101+
}
102+
if newSchema.IsVirtual {
103+
panic(pgerror.Newf(pgcode.FeatureNotSupported,
104+
"cannot move objects into or out of virtual schemas"))
105+
}
106+
}

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/process.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ var supportedStatements = map[reflect.Type]supportedStatement{
7373
reflect.TypeOf((*tree.SetZoneConfig)(nil)): {fn: SetZoneConfig, statementTags: []string{tree.ConfigureZoneTag}, on: true, checks: isV251Active},
7474
reflect.TypeOf((*tree.Truncate)(nil)): {fn: Truncate, statementTags: []string{tree.TruncateTag}, on: true, checks: isV254Active},
7575
reflect.TypeOf((*tree.RenameTable)(nil)): {fn: RenameTable, statementTags: []string{tree.AlterTableTag}, on: true, checks: isV254Active},
76+
reflect.TypeOf((*tree.AlterTableSetSchema)(nil)): {fn: AlterTableSetSchema, statementTags: []string{tree.AlterTableTag}, on: true, checks: isV261Active},
7677
}
7778

7879
// supportedStatementTags tracks statement tags which are implemented

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/rename_table.go

Lines changed: 47 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -32,43 +32,16 @@ func RenameTable(b BuildCtx, n *tree.RenameTable) {
3232
}
3333

3434
// Validate the object type matches what was requested.
35-
validateObjectType(elts, n)
35+
validateObjectType(elts, n.Name.ToTableName().ObjectName, n.IsSequence, n.IsView, n.IsMaterialized)
3636

3737
// Get the fully qualified object name.
38-
objectName := n.Name.ToTableName()
39-
dbElts, scElts := b.ResolveTargetObject(n.Name, privilege.CREATE /* this should be 0 */)
40-
_, _, scName := scpb.FindNamespace(scElts)
41-
_, _, dbname := scpb.FindNamespace(dbElts)
42-
objectName.SchemaName = tree.Name(scName.Name)
43-
objectName.CatalogName = tree.Name(dbname.Name)
44-
objectName.ExplicitCatalog = true
45-
objectName.ExplicitSchema = true
38+
objectName := getAlterTableQualifiedObjectName(n.Name, b)
4639

4740
// Get the descriptor ID for further processing.
48-
var targetDescriptorID catid.DescID
49-
var targetElement scpb.Element
50-
_, _, tbl := scpb.FindTable(elts)
51-
if tbl != nil {
52-
targetElement = tbl
53-
targetDescriptorID = tbl.TableID
54-
}
55-
if n.IsView || targetDescriptorID == 0 {
56-
_, _, view := scpb.FindView(elts)
57-
if view != nil {
58-
targetElement = view
59-
targetDescriptorID = view.ViewID
60-
}
61-
}
62-
if n.IsSequence || targetDescriptorID == 0 {
63-
_, _, seq := scpb.FindSequence(elts)
64-
if seq != nil {
65-
targetElement = seq
66-
targetDescriptorID = seq.SequenceID
67-
}
68-
}
41+
targetDescriptorID, targetElement, _ := getAlterTableTargetElement(elts)
6942

7043
// Check for name-based dependencies that would prevent renaming.
71-
checkNameBasedDependencies(b, targetDescriptorID, targetElement, objectName)
44+
checkNameBasedDependencies(b, targetDescriptorID, targetElement, objectName, "rename")
7245

7346
// Need CREATE privilege on database to match legacy schema changer behavior.
7447
b.ResolveDatabase(objectName.CatalogName, ResolveParams{RequiredPrivilege: privilege.CREATE})
@@ -104,6 +77,7 @@ func RenameTable(b BuildCtx, n *tree.RenameTable) {
10477
newName.ExplicitCatalog = true
10578
newName.ExplicitSchema = true
10679
checkTableNameConflicts(b, objectName, newName, currentNS)
80+
validateTableRename(b, objectName, newName)
10781

10882
// Check if the new name is the same as the old name (no-op case).
10983
if currentNS.Name == string(newName.ObjectName) {
@@ -129,6 +103,18 @@ func RenameTable(b BuildCtx, n *tree.RenameTable) {
129103
b.LogEventForExistingPayload(newNS, renameEvent)
130104
}
131105

106+
func getAlterTableQualifiedObjectName(name *tree.UnresolvedObjectName, b BuildCtx) tree.TableName {
107+
objectName := name.ToTableName()
108+
dbElts, scElts := b.ResolveTargetObject(name, privilege.CREATE /* this should be 0 */)
109+
_, _, scName := scpb.FindNamespace(scElts)
110+
_, _, dbname := scpb.FindNamespace(dbElts)
111+
objectName.SchemaName = tree.Name(scName.Name)
112+
objectName.CatalogName = tree.Name(dbname.Name)
113+
objectName.ExplicitCatalog = true
114+
objectName.ExplicitSchema = true
115+
return objectName
116+
}
117+
132118
// validateTableRename performs validation checks before renaming a table.
133119
func validateTableRename(b BuildCtx, currentName tree.TableName, newName tree.TableName) {
134120
// The legacy schema changer used to check the CREATE privilege on the
@@ -187,34 +173,51 @@ func checkTableNameConflicts(
187173
}
188174
panic(sqlerrors.NewRelationAlreadyExistsError(newName.String()))
189175
}
176+
}
190177

191-
validateTableRename(b, currentName, newName)
178+
func getAlterTableTargetElement(
179+
elts ElementResultSet,
180+
) (descID catid.DescID, element scpb.Element, isTemp bool) {
181+
if tbl := elts.FilterTable().MustGetZeroOrOneElement(); tbl != nil {
182+
element = tbl
183+
descID = tbl.TableID
184+
isTemp = tbl.IsTemporary
185+
} else if view := elts.FilterView().MustGetZeroOrOneElement(); view != nil {
186+
element = view
187+
descID = view.ViewID
188+
isTemp = view.IsTemporary
189+
} else if seq := elts.FilterSequence().MustGetZeroOrOneElement(); seq != nil {
190+
element = seq
191+
descID = seq.SequenceID
192+
isTemp = seq.IsTemporary
193+
}
194+
return descID, element, isTemp
192195
}
193196

194197
// validateObjectType validates that the resolved object type matches what was
195198
// requested in the statement. Note that we allow ALTER TABLE to be used for
196199
// views or sequences, just like in Postgres.
197-
func validateObjectType(elts ElementResultSet, n *tree.RenameTable) {
200+
func validateObjectType(
201+
elts ElementResultSet, objectName tree.Name, isSequence bool, isView bool, isMaterialized bool,
202+
) {
198203
_, _, view := scpb.FindView(elts)
199204
_, _, seq := scpb.FindSequence(elts)
200205

201-
objectName := n.Name.ToTableName().ObjectName
202-
203-
if n.IsView && view == nil {
206+
if isView && view == nil {
204207
// User asked for view but we found something else.
205208
panic(pgerror.Newf(pgcode.WrongObjectType, "%q is not a view", objectName))
206-
} else if n.IsSequence && seq == nil {
209+
} else if isSequence && seq == nil {
207210
// User asked for sequence but we found something else.
208211
panic(pgerror.Newf(pgcode.WrongObjectType, "%q is not a sequence", objectName))
209212
}
210213

211214
if view != nil {
212215
// Validate view type (materialized vs non-materialized).
213-
if view.IsMaterialized && !n.IsMaterialized {
216+
if view.IsMaterialized && !isMaterialized {
214217
panic(errors.WithHint(pgerror.Newf(pgcode.WrongObjectType, "%q is a materialized view", objectName),
215218
"use the corresponding MATERIALIZED VIEW command"))
216219
}
217-
if !view.IsMaterialized && n.IsMaterialized {
220+
if !view.IsMaterialized && isMaterialized {
218221
panic(pgerror.Newf(pgcode.WrongObjectType, "%q is not a materialized view", objectName))
219222
}
220223
}
@@ -223,7 +226,7 @@ func validateObjectType(elts ElementResultSet, n *tree.RenameTable) {
223226
// checkNameBasedDependencies validates that no objects depend on this object
224227
// via its name.
225228
func checkNameBasedDependencies(
226-
b BuildCtx, descriptorID catid.DescID, element scpb.Element, objectName tree.TableName,
229+
b BuildCtx, descriptorID catid.DescID, element scpb.Element, objectName tree.TableName, op string,
227230
) {
228231
switch element.(type) {
229232
case *scpb.Sequence:
@@ -238,14 +241,14 @@ func checkNameBasedDependencies(
238241
// blocked.
239242
viewElts := b.QueryByID(backRefElem.ViewID)
240243
_, _, viewNS := scpb.FindNamespace(viewElts)
241-
panic(sqlerrors.NewDependentBlocksOpError("rename", "relation", objectName.String(), "view", viewNS.Name))
244+
panic(sqlerrors.NewDependentBlocksOpError(op, "relation", objectName.String(), "view", viewNS.Name))
242245
case *scpb.FunctionName:
243246
funcElem := b.QueryByID(backRefElem.FunctionID).FilterFunction().MustGetOneElement()
244247
funcType := "function"
245248
if funcElem.IsProcedure {
246249
funcType = "procedure"
247250
}
248-
panic(sqlerrors.NewDependentBlocksOpError("rename", "relation", objectName.String(), funcType, backRefElem.Name))
251+
panic(sqlerrors.NewDependentBlocksOpError(op, "relation", objectName.String(), funcType, backRefElem.Name))
249252
case *scpb.TriggerDeps:
250253
for _, usesRelation := range backRefElem.UsesRelations {
251254
if usesRelation.ID == descriptorID {
@@ -255,9 +258,8 @@ func checkNameBasedDependencies(
255258
dependentTriggerName := backRefs.FilterTriggerName().Filter(func(_ scpb.Status, _ scpb.TargetStatus, e *scpb.TriggerName) bool {
256259
return e.TriggerID == dependentTriggerID && e.TableID == dependentTableID
257260
}).MustGetOneElement()
258-
panic(sqlerrors.NewDependentObjectErrorf(
259-
"cannot rename relation %q because trigger %q on table %q depends on it",
260-
objectName.String(), dependentTriggerName.Name, dependentTableNS.Name,
261+
panic(sqlerrors.NewDependentBlocksOpError(
262+
op, "relation", objectName.String(), dependentTriggerName.Name, dependentTableNS.Name,
261263
))
262264
}
263265
}

0 commit comments

Comments
 (0)