Skip to content

Commit 44921a5

Browse files
craig[bot]rafissbghalspilchen
committed
152895: schemachanger: implement ALTER TABLE .. RENAME in declarative schemachanger r=rafiss a=rafiss See individual commits to make this easier to review. This preserves all the validations and error handling from the legacy schema changer. ### scexec: add deferred mutation to rename TTL schedule This will make sure the TTL schedule name matches the new table name when a table is renamed, just as the legacy schema changer does. ### scbuild: handle trigger/procedure dependencies for table renames ### lease_test: fix tests that rely on legacy testing knobs Since RENAME now works in the declarative schema changer, we need to force these tests to use the legacy schema changer to test the desired behavior. fixes #148339 Release note: None 153737: sql: make inspect a system privilege r=bghal a=bghal Previously the inspect privilege was tied to specific objects (tables and databases). For simplicity, it has been changed to a system privilege. This has no user impact as the commands associated with the privilege are as yet unimplemented. Epic: CRDB-30356 Part of: #148925 Release note: None 153787: sql/schemachanger: fix lost dependencies in ALTER POLICY expressions r=spilchen a=spilchen When altering only the USING expression of a policy that also had a WITH CHECK expression, dependencies from the WITH CHECK expression were incorrectly lost. This occurred because the upsertPolicyExpressions function overwrote dependency sets instead of unioning them. Fixes #153191 Epic: none Release note (bug fix): Fixed ALTER POLICY incorrectly dropping dependency tracking for functions, sequences, or types in policy expressions. Co-authored-by: Rafi Shamim <[email protected]> Co-authored-by: Brendan Gerrity <[email protected]> Co-authored-by: Matt Spilchen <[email protected]>
4 parents 5feef23 + dc7e101 + 755d9e8 + b2ae661 commit 44921a5

File tree

78 files changed

+791
-126
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

78 files changed

+791
-126
lines changed

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,11 +182,10 @@ DROP TABLE sc1.tbl1
182182
----
183183
pq: cannot drop table tbl1 because other objects depend on it
184184

185-
# TODO(mgartner): The error message should say "procedure".
186185
exec-sql
187186
ALTER TABLE sc1.tbl1 RENAME TO tbl1_new
188187
----
189-
pq: cannot rename relation "sc1.tbl1" because function "p1" depends on it
188+
pq: cannot rename relation "db1_new.sc1.tbl1" because procedure "p1" depends on it
190189
HINT: consider dropping "p1" first.
191190

192191
# TODO(mgartner): The error message should say "procedure".
@@ -384,7 +383,7 @@ pq: cannot drop table tbl1 because other objects depend on it
384383
exec-sql
385384
ALTER TABLE sc1.tbl1 RENAME TO tbl1_new
386385
----
387-
pq: cannot rename relation "sc1.tbl1" because function "p1" depends on it
386+
pq: cannot rename relation "db1.sc1.tbl1" because procedure "p1" depends on it
388387
HINT: consider dropping "p1" first.
389388

390389
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
@@ -247,7 +247,7 @@ pq: cannot drop table tbl1 because other objects depend on it
247247
exec-sql
248248
ALTER TABLE sc1.tbl1 RENAME TO tbl1_new
249249
----
250-
pq: cannot rename relation "sc1.tbl1" because function "f1" depends on it
250+
pq: cannot rename relation "db1_new.sc1.tbl1" because function "f1" depends on it
251251
HINT: consider dropping "f1" first.
252252

253253
exec-sql
@@ -463,7 +463,7 @@ pq: cannot drop table tbl1 because other objects depend on it
463463
exec-sql
464464
ALTER TABLE sc1.tbl1 RENAME TO tbl1_new
465465
----
466-
pq: cannot rename relation "sc1.tbl1" because function "f1" depends on it
466+
pq: cannot rename relation "db1.sc1.tbl1" because function "f1" depends on it
467467
HINT: consider dropping "f1" first.
468468

469469
exec-sql

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,11 +168,10 @@ DROP TABLE sc1.tbl1
168168
----
169169
pq: cannot drop table tbl1 because other objects depend on it
170170

171-
# TODO(mgartner): The error message should say "procedure".
172171
exec-sql
173172
ALTER TABLE sc1.tbl1 RENAME TO tbl1_new
174173
----
175-
pq: cannot rename relation "sc1.tbl1" because function "p1" depends on it
174+
pq: cannot rename relation "db1_new.sc1.tbl1" because procedure "p1" depends on it
176175
HINT: consider dropping "p1" first.
177176

178177
# TODO(mgartner): The error message should say "procedure".
@@ -356,7 +355,7 @@ pq: cannot drop table tbl1 because other objects depend on it
356355
exec-sql
357356
ALTER TABLE sc1.tbl1 RENAME TO tbl1_new
358357
----
359-
pq: cannot rename relation "sc1.tbl1" because function "p1" depends on it
358+
pq: cannot rename relation "db1.sc1.tbl1" because procedure "p1" depends on it
360359
HINT: consider dropping "p1" first.
361360

362361
exec-sql

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ pq: cannot drop table tbl1 because other objects depend on it
218218
exec-sql
219219
ALTER TABLE sc1.tbl1 RENAME TO tbl1_new
220220
----
221-
pq: cannot rename relation "sc1.tbl1" because trigger "tr1" on table "tbl1" depends on it
221+
pq: cannot rename relation "db1_new.sc1.tbl1" because trigger "tr1" on table "tbl1" depends on it
222222

223223
exec-sql
224224
ALTER TABLE sc1.tbl1 SET SCHEMA sc2;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ pq: cannot drop table tbl1 because other objects depend on it
165165
exec-sql
166166
ALTER TABLE sc1.tbl1 RENAME TO tbl1_new
167167
----
168-
pq: cannot rename relation "sc1.tbl1" because function "f1" depends on it
168+
pq: cannot rename relation "db1_new.sc1.tbl1" because function "f1" depends on it
169169
HINT: consider dropping "f1" first.
170170

171171
exec-sql
@@ -352,7 +352,7 @@ pq: cannot drop table tbl1 because other objects depend on it
352352
exec-sql
353353
ALTER TABLE sc1.tbl1 RENAME TO tbl1_new
354354
----
355-
pq: cannot rename relation "sc1.tbl1" because function "f1" depends on it
355+
pq: cannot rename relation "db1.sc1.tbl1" because function "f1" depends on it
356356
HINT: consider dropping "f1" first.
357357

358358
exec-sql

‎pkg/ccl/changefeedccl/alter_changefeed_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1273,7 +1273,8 @@ func TestAlterChangefeedAlterTableName(t *testing.T) {
12731273

12741274
expectResolvedTimestamp(t, testFeed)
12751275

1276-
waitForSchemaChange(t, sqlDB, `ALTER TABLE movr.users RENAME TO movr.riders`)
1276+
sqlDB.Exec(t, `ALTER TABLE movr.users RENAME TO movr.riders`)
1277+
sqlDB.CheckQueryResultsRetry(t, "SELECT count(*) FROM [SHOW TABLES FROM movr] WHERE table_name = 'riders'", [][]string{{"1"}})
12771278

12781279
var tsLogical string
12791280
sqlDB.QueryRow(t, `SELECT cluster_logical_timestamp()`).Scan(&tsLogical)

‎pkg/ccl/logictestccl/testdata/logic_test/triggers‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -929,9 +929,14 @@ statement ok
929929
CREATE TRIGGER foo AFTER INSERT ON xy FOR EACH ROW EXECUTE FUNCTION trigger_func();
930930

931931
# Relations are referenced by name, so renaming the table is not allowed.
932+
onlyif config local-legacy-schema-changer local-mixed-25.2 local-mixed-25.3
932933
statement error pgcode 2BP01 cannot rename relation "t" because trigger "foo" on table "xy" depends on it
933934
ALTER TABLE t RENAME TO t2;
934935

936+
skipif config local-legacy-schema-changer local-mixed-25.2 local-mixed-25.3
937+
statement error pgcode 2BP01 cannot rename relation "test.public.t" because trigger "foo" on table "xy" depends on it
938+
ALTER TABLE t RENAME TO t2;
939+
935940
# Sequences are remapped to their IDs, so renaming is allowed.
936941
statement ok
937942
ALTER SEQUENCE s RENAME TO s2;

‎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/catalog/catpb/privilege_test.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@ func TestPrivilege(t *testing.T) {
8787
{Kind: privilege.CREATE},
8888
{Kind: privilege.DELETE},
8989
{Kind: privilege.DROP},
90-
{Kind: privilege.INSPECT},
9190
{Kind: privilege.REPLICATIONDEST},
9291
{Kind: privilege.REPLICATIONSOURCE},
9392
{Kind: privilege.TRIGGER},
@@ -616,7 +615,7 @@ func TestRevokeWithGrantOption(t *testing.T) {
616615
true,
617616
privilege.List{privilege.CREATE},
618617
privilege.List{privilege.ALL},
619-
privilege.List{privilege.BACKUP, privilege.CHANGEFEED, privilege.DELETE, privilege.DROP, privilege.INSERT, privilege.INSPECT, privilege.REPLICATIONDEST, privilege.REPLICATIONSOURCE, privilege.SELECT, privilege.TRIGGER, privilege.UPDATE, privilege.ZONECONFIG},
618+
privilege.List{privilege.BACKUP, privilege.CHANGEFEED, privilege.DELETE, privilege.DROP, privilege.INSERT, privilege.REPLICATIONDEST, privilege.REPLICATIONSOURCE, privilege.SELECT, privilege.TRIGGER, privilege.UPDATE, privilege.ZONECONFIG},
620619
false},
621620
{catpb.NewPrivilegeDescriptor(testUser, privilege.List{privilege.ALL}, privilege.List{privilege.ALL}, username.AdminRoleName()),
622621
testUser, privilege.Table,
@@ -650,8 +649,8 @@ func TestRevokeWithGrantOption(t *testing.T) {
650649
testUser, privilege.Table,
651650
false,
652651
privilege.List{privilege.CREATE},
653-
privilege.List{privilege.BACKUP, privilege.CHANGEFEED, privilege.DELETE, privilege.DROP, privilege.INSERT, privilege.INSPECT, privilege.REPLICATIONDEST, privilege.REPLICATIONSOURCE, privilege.SELECT, privilege.TRIGGER, privilege.UPDATE, privilege.ZONECONFIG},
654-
privilege.List{privilege.BACKUP, privilege.CHANGEFEED, privilege.DELETE, privilege.DROP, privilege.INSERT, privilege.INSPECT, privilege.REPLICATIONDEST, privilege.REPLICATIONSOURCE, privilege.SELECT, privilege.TRIGGER, privilege.UPDATE, privilege.ZONECONFIG},
652+
privilege.List{privilege.BACKUP, privilege.CHANGEFEED, privilege.DELETE, privilege.DROP, privilege.INSERT, privilege.REPLICATIONDEST, privilege.REPLICATIONSOURCE, privilege.SELECT, privilege.TRIGGER, privilege.UPDATE, privilege.ZONECONFIG},
653+
privilege.List{privilege.BACKUP, privilege.CHANGEFEED, privilege.DELETE, privilege.DROP, privilege.INSERT, privilege.REPLICATIONDEST, privilege.REPLICATIONSOURCE, privilege.SELECT, privilege.TRIGGER, privilege.UPDATE, privilege.ZONECONFIG},
655654
false},
656655
{catpb.NewPrivilegeDescriptor(testUser, privilege.List{privilege.SELECT, privilege.INSERT}, privilege.List{privilege.INSERT}, username.AdminRoleName()),
657656
testUser, privilege.Table,

‎pkg/sql/catalog/lease/lease_test.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1298,7 +1298,12 @@ func TestIncrementTableVersion(t *testing.T) {
12981298
defer srv.Stopper().Stop(context.Background())
12991299
codec := srv.ApplicationLayer().Codec()
13001300

1301+
// This test relies on legacy schema changer testing knobs.
1302+
if _, err := sqlDB.Exec(`SET CLUSTER SETTING sql.defaults.use_declarative_schema_changer = 'off';`); err != nil {
1303+
t.Fatal(err)
1304+
}
13011305
if _, err := sqlDB.Exec(`
1306+
SET use_declarative_schema_changer = 'off';
13021307
CREATE DATABASE t;
13031308
CREATE TABLE t.kv (k CHAR PRIMARY KEY, v CHAR);
13041309
`); err != nil {
@@ -1404,7 +1409,12 @@ func TestTwoVersionInvariantRetryErrorWithSavePoint(t *testing.T) {
14041409
defer srv.Stopper().Stop(context.Background())
14051410
s := srv.ApplicationLayer()
14061411

1412+
// This test relies on legacy schema changer testing knobs.
1413+
if _, err := sqlDB.Exec(`SET CLUSTER SETTING sql.defaults.use_declarative_schema_changer = 'off';`); err != nil {
1414+
t.Fatal(err)
1415+
}
14071416
if _, err := sqlDB.Exec(`
1417+
SET use_declarative_schema_changer = 'off';
14081418
CREATE DATABASE t;
14091419
CREATE TABLE t.kv (k CHAR PRIMARY KEY, v CHAR);
14101420
INSERT INTO t.kv VALUES ('a', 'b');
@@ -1524,7 +1534,12 @@ func TestTwoVersionInvariantRetryError(t *testing.T) {
15241534
defer srv.Stopper().Stop(context.Background())
15251535
s := srv.ApplicationLayer()
15261536

1537+
// This test relies on legacy schema changer testing knobs.
1538+
if _, err := sqlDB.Exec(`SET CLUSTER SETTING sql.defaults.use_declarative_schema_changer = 'off';`); err != nil {
1539+
t.Fatal(err)
1540+
}
15271541
if _, err := sqlDB.Exec(`
1542+
SET use_declarative_schema_changer = 'off';
15281543
CREATE DATABASE t;
15291544
CREATE TABLE t.kv (k CHAR PRIMARY KEY, v CHAR);
15301545
INSERT INTO t.kv VALUES ('a', 'b');

0 commit comments

Comments
 (0)