Skip to content

Commit bc3f228

Browse files
rafissclaude
andcommitted
sql/sem/tree: prevent schema_locked bypass with comma syntax
Previously, the legacy schema changer allowed bypassing the schema_locked protection by combining SET (schema_locked=false) with other schema-changing commands in the same ALTER TABLE statement. For example: ALTER TABLE t SET (schema_locked=false), DROP COLUMN n; This would first unset schema_locked, then execute the DROP COLUMN on the now-unlocked table, effectively circumventing the protection. This commit fixes `IsSetOrResetSchemaLocked()` to return false when the ALTER TABLE statement contains any commands other than storage parameter changes. This ensures schema_locked changes cannot be combined with other schema changes in the same statement. Fixes: #150484 Release note (bug fix): Fixed a bug where the `schema_locked` table storage parameter could be bypassed by combining `SET (schema_locked=false)` with other schema changes in the same `ALTER TABLE` statement using comma syntax. Schema-locked tables now correctly reject such combined statements. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent fc63227 commit bc3f228

File tree

2 files changed

+55
-5
lines changed

2 files changed

+55
-5
lines changed

pkg/sql/logictest/testdata/logic_test/schema_locked

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,3 +218,45 @@ t_147993 CREATE TABLE public.t_147993 (
218218
) WITH (schema_locked = true);
219219

220220
subtest end
221+
222+
# Regression test for #150484: schema_locked cannot be unset with the comma
223+
# syntax combined with other schema-changing operations. This prevents bypassing
224+
# the schema_locked protection by unsetting the lock and performing a schema
225+
# change in the same ALTER TABLE statement.
226+
subtest regression_150484
227+
228+
statement ok
229+
CREATE TABLE t_150484 (n INT, m INT, FAMILY (n, m)) WITH (schema_locked=true);
230+
231+
# Combining SET (schema_locked=false) with DROP COLUMN should fail.
232+
statement error pgcode 57000 pq: this schema change is disallowed because table "t_150484" is locked
233+
ALTER TABLE t_150484 SET (schema_locked=false), DROP COLUMN n;
234+
235+
# Combining RESET (schema_locked) with ADD COLUMN should fail.
236+
statement error pgcode 57000 pq: this schema change is disallowed because table "t_150484" is locked
237+
ALTER TABLE t_150484 RESET (schema_locked), ADD COLUMN k INT;
238+
239+
# Verify the table is still locked and unchanged.
240+
query TT
241+
SHOW CREATE TABLE t_150484
242+
----
243+
t_150484 CREATE TABLE public.t_150484 (
244+
n INT8 NULL,
245+
m INT8 NULL,
246+
rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(),
247+
CONSTRAINT t_150484_pkey PRIMARY KEY (rowid ASC),
248+
FAMILY fam_0_n_m_rowid (n, m, rowid)
249+
) WITH (schema_locked = true);
250+
251+
# Confirm that SET (schema_locked=false) alone still works.
252+
statement ok
253+
ALTER TABLE t_150484 SET (schema_locked=false);
254+
255+
# Now schema changes should work.
256+
statement ok
257+
ALTER TABLE t_150484 DROP COLUMN n;
258+
259+
statement ok
260+
DROP TABLE t_150484;
261+
262+
subtest end

pkg/sql/sem/tree/schema_helpers.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,26 +7,34 @@ package tree
77

88
import "slices"
99

10-
// IsSetOrResetSchemaLocked returns true if `n` contains a command to
11-
// set/reset "schema_locked" storage parameter.
10+
// IsSetOrResetSchemaLocked returns true if `n` contains only commands to
11+
// set/reset storage parameters including "schema_locked", with no other
12+
// schema-changing commands. This prevents bypassing schema_locked protection
13+
// by combining SET (schema_locked=false) with other operations like DROP
14+
// COLUMN in the same ALTER TABLE statement.
1215
func IsSetOrResetSchemaLocked(n Statement) bool {
1316
alterStmt, ok := n.(*AlterTable)
1417
if !ok {
1518
return false
1619
}
20+
hasSchemaLockedChange := false
1721
for _, cmd := range alterStmt.Cmds {
1822
switch cmd := cmd.(type) {
1923
case *AlterTableSetStorageParams:
2024
if cmd.StorageParams.GetVal("schema_locked") != nil {
21-
return true
25+
hasSchemaLockedChange = true
2226
}
2327
case *AlterTableResetStorageParams:
2428
if slices.Contains(cmd.Params, "schema_locked") {
25-
return true
29+
hasSchemaLockedChange = true
2630
}
31+
default:
32+
// Any other command type means this is not a pure storage parameter
33+
// change, so we should not allow bypassing schema_locked checks.
34+
return false
2735
}
2836
}
29-
return false
37+
return hasSchemaLockedChange
3038
}
3139

3240
// IsAllowedLDRSchemaChange returns true if the schema change statement is

0 commit comments

Comments
 (0)