Skip to content

Commit 35625cf

Browse files
craig[bot]annrpomrafissarulajmani
committed
152769: schemachanger: implement RENAME COLUMN in the declarative schema changer r=rafiss a=rafiss This was a pretty straightforward copy of the logic from the legacy schema changer. Only one new dependency rule was needed, to make sure the old column name is removed before the new one is added. fixes #148340 Release note: None 152884: kvserver: fix TestUpdateLastUpdateTimesUsingStoreLiveness r=arulajmani a=arulajmani This test wasn't doing as advertised on the tin after I broke some testing knobs in 02fa61b. Epic: none Release note: None Co-authored-by: Annie Pompa <[email protected]> Co-authored-by: Rafi Shamim <[email protected]> Co-authored-by: Arul Ajmani <[email protected]>
3 parents 84697f3 + 53017c1 + 4837dbc commit 35625cf

31 files changed

+661
-51
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/kv/kvserver/replica_raft.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -699,7 +699,7 @@ func (r *Replica) stepRaftGroupRaftMuLocked(req *kvserverpb.RaftMessageRequest)
699699
// Update the lastUpdateTimes map, unless configured not to by a testing
700700
// knob.
701701
disableUpdateLastUpdateTimesMapOnRaftGroupStep := false
702-
if r.store.TestingKnobs() == nil &&
702+
if r.store.TestingKnobs() != nil &&
703703
r.store.TestingKnobs().DisableUpdateLastUpdateTimesMapOnRaftGroupStep != nil {
704704
disableUpdateLastUpdateTimesMapOnRaftGroupStep = r.store.TestingKnobs().DisableUpdateLastUpdateTimesMapOnRaftGroupStep(r)
705705
}

pkg/sql/alter_table.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -804,7 +804,7 @@ func (n *alterTableNode) startExec(params runParams) error {
804804
tableDesc.GetRowLevelTTL().HasDurationExpr() {
805805
return pgerror.Newf(
806806
pgcode.InvalidTableDefinition,
807-
`cannot rename column %s while ttl_expire_after is set`,
807+
`cannot alter column %s while ttl_expire_after is set`,
808808
columnName,
809809
)
810810
}
@@ -986,7 +986,7 @@ func applyColumnMutation(
986986
}
987987
return pgerror.Newf(
988988
pgcode.Syntax,
989-
"computed column %q cannot also have a DEFAULT expression",
989+
"computed column %q cannot also have a DEFAULT or ON UPDATE expression",
990990
col.GetName())
991991
}
992992
if err := updateNonComputedColExpr(
@@ -1005,6 +1005,12 @@ func applyColumnMutation(
10051005
}
10061006

10071007
case *tree.AlterTableSetOnUpdate:
1008+
if col.IsComputed() {
1009+
return pgerror.Newf(
1010+
pgcode.Syntax,
1011+
"computed column %q cannot also have a DEFAULT or ON UPDATE expression",
1012+
col.GetName())
1013+
}
10081014
// We want to reject uses of ON UPDATE where there is also a foreign key ON
10091015
// UPDATE.
10101016
for _, fk := range tableDesc.OutboundFKs {

pkg/sql/catalog/tabledesc/structured_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -963,7 +963,7 @@ func TestRemoveDefaultExprFromComputedColumn(t *testing.T) {
963963
defer srv.Stopper().Stop(context.Background())
964964
tdb := sqlutils.MakeSQLRunner(sqlDB)
965965

966-
const expectedErrRE = `.*: computed column \"b\" cannot also have a DEFAULT expression`
966+
const expectedErrRE = `.*: computed column \"b\" cannot also have a DEFAULT or ON UPDATE expression`
967967
// Create a table with a computed column.
968968
tdb.Exec(t, `CREATE DATABASE t`)
969969
tdb.Exec(t, `CREATE TABLE t.tbl (a INT PRIMARY KEY, b INT AS (1) STORED)`)

pkg/sql/logictest/testdata/logic_test/computed

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1319,7 +1319,7 @@ SELECT * FROM t69327
13191319
f f
13201320

13211321
# Regression test for #72881. Computed columns can't have a DEFAULT expr.
1322-
statement error pgcode 42601 computed column "v" cannot also have a DEFAULT expression
1322+
statement error pgcode 42601 computed column "v" cannot also have a DEFAULT or ON UPDATE expression
13231323
ALTER TABLE t69327 ALTER COLUMN v SET DEFAULT 'foo'
13241324

13251325
# Regression test for #69665.Computed columns should be evaluated after
@@ -1379,10 +1379,13 @@ CREATE TABLE foooooo (
13791379
gen INT AS (x + y) STORED
13801380
);
13811381

1382-
statement error pgcode 42601 computed column "gen" cannot also have a DEFAULT expression
1382+
statement error pgcode 42601 computed column "gen" cannot also have a DEFAULT or ON UPDATE expression
13831383
ALTER TABLE foooooo ALTER COLUMN gen SET DEFAULT 1;
13841384

1385-
statement error pgcode 42601 computed column "gen" cannot also have a DEFAULT expression
1385+
statement error pgcode 42601 computed column "gen" cannot also have a DEFAULT or ON UPDATE expression
1386+
ALTER TABLE foooooo ALTER COLUMN gen SET ON UPDATE 1;
1387+
1388+
statement error pgcode 42601 computed column "gen" cannot also have a DEFAULT or ON UPDATE expression
13861389
ALTER TABLE foooooo ALTER COLUMN gen SET DEFAULT NULL;
13871390

13881391
statement error pgcode 42601 column "gen" of relation "foooooo" is a computed column

pkg/sql/logictest/testdata/logic_test/rename_column

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,8 @@ ALTER TABLE users RENAME COLUMN id TO uid
9292
statement error cannot rename column "username" because view "v1" depends on it
9393
ALTER TABLE users RENAME COLUMN username TO name
9494

95-
# TODO(knz): restore test after #17269 / #10083 is fixed.
96-
#statement ok
97-
#ALTER TABLE users RENAME COLUMN species TO title
95+
statement ok
96+
ALTER TABLE users RENAME COLUMN species TO title
9897

9998
statement ok
10099
CREATE VIEW v2 AS SELECT id from users
@@ -105,9 +104,8 @@ DROP VIEW v1
105104
statement error cannot rename column "id" because view "v2" depends on it
106105
ALTER TABLE users RENAME COLUMN id TO uid
107106

108-
# TODO(knz): restore test after #17269 / #10083 is fixed.
109-
# statement ok
110-
# ALTER TABLE users RENAME COLUMN username TO name
107+
statement ok
108+
ALTER TABLE users RENAME COLUMN username TO name
111109

112110
statement ok
113111
DROP VIEW v2
@@ -116,14 +114,14 @@ query T
116114
SELECT column_name FROM [SHOW COLUMNS FROM users] ORDER BY column_name
117115
----
118116
id
119-
species
120-
username
117+
name
118+
title
121119

122120
statement ok
123121
SET vectorize=on
124122

125123
query T
126-
EXPLAIN ALTER TABLE users RENAME COLUMN species TO woo
124+
EXPLAIN ALTER TABLE users RENAME COLUMN title TO woo
127125
----
128126
distribution: local
129127
vectorized: true
@@ -138,24 +136,24 @@ query T
138136
SELECT column_name FROM [SHOW COLUMNS FROM users] ORDER BY column_name
139137
----
140138
id
141-
species
142-
username
139+
name
140+
title
143141

144142
skipif config schema-locked-disabled
145143
statement ok
146144
ALTER TABLE users SET (schema_locked=false);
147145

148146
# Check that a column can be added and renamed in the same statement
149147
statement ok
150-
ALTER TABLE users RENAME COLUMN species TO species_old,
151-
ADD COLUMN species STRING AS (species_old || ' woo') STORED
148+
ALTER TABLE users RENAME COLUMN title TO title_old,
149+
ADD COLUMN title STRING AS (title_old || ' woo') STORED
152150

153151
skipif config schema-locked-disabled
154152
statement ok
155153
ALTER TABLE users SET (schema_locked=true);
156154

157155
query T rowsort
158-
SELECT species FROM users
156+
SELECT title FROM users
159157
----
160158
cat woo
161159
rat woo
@@ -199,3 +197,34 @@ query II
199197
SELECT j, k FROM foo;
200198
----
201199
1 2
200+
201+
# Test that mixed-case column names are handled correctly for rename operations.
202+
statement ok
203+
CREATE TABLE mixed_case_table (
204+
"CamelCase" INT PRIMARY KEY,
205+
"snake_case" TEXT,
206+
"UPPERCASE" DECIMAL
207+
);
208+
209+
statement error column "UPPERCASE" of relation "mixed_case_table" already exists
210+
ALTER TABLE mixed_case_table RENAME COLUMN "CamelCase" TO "UPPERCASE";
211+
212+
statement ok
213+
ALTER TABLE mixed_case_table RENAME COLUMN "CamelCase" TO "CamelCase";
214+
215+
statement ok
216+
ALTER TABLE mixed_case_table RENAME COLUMN "CamelCase" TO "NewCamelCase";
217+
218+
statement ok
219+
ALTER TABLE mixed_case_table RENAME COLUMN "snake_case" TO "SnakeCase";
220+
221+
statement ok
222+
ALTER TABLE mixed_case_table RENAME COLUMN "UPPERCASE" TO "decimal_value";
223+
224+
query T colnames
225+
SELECT column_name FROM [SHOW COLUMNS FROM mixed_case_table] ORDER BY column_name;
226+
----
227+
column_name
228+
NewCamelCase
229+
SnakeCase
230+
decimal_value

pkg/sql/logictest/testdata/logic_test/row_level_ttl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ subtest alter_column_crdb_internal_expiration_rename
227227
statement ok
228228
CREATE TABLE alter_column_crdb_internal_expiration_rename() WITH (ttl_expire_after='10 minutes')
229229

230-
statement error cannot rename column crdb_internal_expiration while ttl_expire_after is set
230+
statement error cannot alter column crdb_internal_expiration while ttl_expire_after is set
231231
ALTER TABLE alter_column_crdb_internal_expiration_rename RENAME COLUMN crdb_internal_expiration TO crdb_internal_expiration_2
232232

233233
subtest end

pkg/sql/logictest/testdata/logic_test/system_columns

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ ALTER TABLE tab3 SET (schema_locked=true)
253253

254254
subtest alter_commands
255255

256-
statement error pq: cannot rename system column "crdb_internal_mvcc_timestamp"
256+
statement error pq: cannot alter system column "crdb_internal_mvcc_timestamp"
257257
ALTER TABLE tab3 RENAME crdb_internal_mvcc_timestamp TO blah;
258258

259259
statement error pq: cannot alter system column "crdb_internal_mvcc_timestamp"

pkg/sql/rename_column.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func (p *planner) findColumnToRename(
105105
if col.IsSystemColumn() {
106106
return nil, pgerror.Newf(
107107
pgcode.FeatureNotSupported,
108-
"cannot rename system column %q", col.ColName(),
108+
"cannot alter system column %q", col.ColName(),
109109
)
110110
}
111111
for _, tableRef := range tableDesc.DependedOnBy {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ go_library(
1515
"alter_table_alter_primary_key.go",
1616
"alter_table_drop_column.go",
1717
"alter_table_drop_constraint.go",
18+
"alter_table_rename_column.go",
1819
"alter_table_set_rls_mode.go",
1920
"alter_table_validate_constraint.go",
2021
"comment_on.go",

0 commit comments

Comments
 (0)