Skip to content

Commit cd6cc7a

Browse files
shghasemishghasemi
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 cd6cc7a

File tree

17 files changed

+373
-57
lines changed

17 files changed

+373
-57
lines changed

‎pkg/backup/testdata/backup-restore/plpgsql_procedures‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,11 +188,10 @@ ALTER TABLE sc1.tbl1 RENAME TO tbl1_new
188188
pq: cannot rename relation "db1_new.sc1.tbl1" because procedure "p1" depends on it
189189
HINT: consider dropping "p1" first.
190190

191-
# TODO(mgartner): The error message should say "procedure".
192191
exec-sql
193192
ALTER TABLE sc1.tbl1 SET SCHEMA sc2;
194193
----
195-
pq: cannot set schema on relation "tbl1" because function "p1" depends on it
194+
pq: cannot set schema on relation "db1_new.sc1.tbl1" because procedure "p1" depends on it
196195
HINT: consider dropping "p1" first.
197196

198197
exec-sql
@@ -389,7 +388,7 @@ HINT: consider dropping "p1" first.
389388
exec-sql
390389
ALTER TABLE sc1.tbl1 SET SCHEMA sc2;
391390
----
392-
pq: cannot set schema on relation "tbl1" because function "p1" depends on it
391+
pq: cannot set schema on relation "db1.sc1.tbl1" because procedure "p1" depends on it
393392
HINT: consider dropping "p1" first.
394393

395394
exec-sql

‎pkg/backup/testdata/backup-restore/plpgsql_user_defined_functions‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ HINT: consider dropping "f1" first.
253253
exec-sql
254254
ALTER TABLE sc1.tbl1 SET SCHEMA sc2;
255255
----
256-
pq: cannot set schema on relation "tbl1" because function "f1" depends on it
256+
pq: cannot set schema on relation "db1_new.sc1.tbl1" because function "f1" depends on it
257257
HINT: consider dropping "f1" first.
258258

259259
exec-sql
@@ -469,7 +469,7 @@ HINT: consider dropping "f1" first.
469469
exec-sql
470470
ALTER TABLE sc1.tbl1 SET SCHEMA sc2;
471471
----
472-
pq: cannot set schema on relation "tbl1" because function "f1" depends on it
472+
pq: cannot set schema on relation "db1.sc1.tbl1" because function "f1" depends on it
473473
HINT: consider dropping "f1" first.
474474

475475
exec-sql

‎pkg/backup/testdata/backup-restore/procedures‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -174,11 +174,10 @@ ALTER TABLE sc1.tbl1 RENAME TO tbl1_new
174174
pq: cannot rename relation "db1_new.sc1.tbl1" because procedure "p1" depends on it
175175
HINT: consider dropping "p1" first.
176176

177-
# TODO(mgartner): The error message should say "procedure".
178177
exec-sql
179178
ALTER TABLE sc1.tbl1 SET SCHEMA sc2;
180179
----
181-
pq: cannot set schema on relation "tbl1" because function "p1" depends on it
180+
pq: cannot set schema on relation "db1_new.sc1.tbl1" because procedure "p1" depends on it
182181
HINT: consider dropping "p1" first.
183182

184183
exec-sql
@@ -361,7 +360,7 @@ HINT: consider dropping "p1" first.
361360
exec-sql
362361
ALTER TABLE sc1.tbl1 SET SCHEMA sc2;
363362
----
364-
pq: cannot set schema on relation "tbl1" because function "p1" depends on it
363+
pq: cannot set schema on relation "db1.sc1.tbl1" because procedure "p1" depends on it
365364
HINT: consider dropping "p1" first.
366365

367366
exec-sql

‎pkg/backup/testdata/backup-restore/triggers‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ pq: cannot rename relation "db1_new.sc1.tbl1" because trigger "tr1" on table "tb
223223
exec-sql
224224
ALTER TABLE sc1.tbl1 SET SCHEMA sc2;
225225
----
226-
pq: cannot set schema on relation "tbl1" because trigger "tr1" on table "tbl1" depends on it
226+
pq: cannot set schema on relation "db1_new.sc1.tbl1" because trigger "tr1" on table "tbl1" depends on it
227227

228228
exec-sql
229229
DROP TYPE sc1.enum1

‎pkg/backup/testdata/backup-restore/user-defined-functions‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ HINT: consider dropping "f1" first.
171171
exec-sql
172172
ALTER TABLE sc1.tbl1 SET SCHEMA sc2;
173173
----
174-
pq: cannot set schema on relation "tbl1" because function "f1" depends on it
174+
pq: cannot set schema on relation "db1_new.sc1.tbl1" because function "f1" depends on it
175175
HINT: consider dropping "f1" first.
176176

177177
exec-sql
@@ -358,7 +358,7 @@ HINT: consider dropping "f1" first.
358358
exec-sql
359359
ALTER TABLE sc1.tbl1 SET SCHEMA sc2;
360360
----
361-
pq: cannot set schema on relation "tbl1" because function "f1" depends on it
361+
pq: cannot set schema on relation "db1.sc1.tbl1" because function "f1" depends on it
362362
HINT: consider dropping "f1" first.
363363

364364
exec-sql

‎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",

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

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

0 commit comments

Comments
 (0)