Skip to content

Commit 33fbda1

Browse files
committed
sql: add ALTER TABLE ... DROP STORED to the declerative schema changer
Previously, `ALTER TABLE ... DROP STORED` would run with the legacy schema changer. This change enables the comment be run by the declerative schema changer. Epic: none Fixes: #139603 Release note (sql change): `ALTER TABLE ... DROP STORED` will use the declerative schema changer to run.
1 parent 5fdc01e commit 33fbda1

12 files changed

+371
-4
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/schemachanger/scbuild/internal/scbuildstmt/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ go_library(
88
"alter_table_add_column.go",
99
"alter_table_add_constraint.go",
1010
"alter_table_alter_column_drop_not_null.go",
11+
"alter_table_alter_column_drop_stored.go",
1112
"alter_table_alter_column_set_default.go",
1213
"alter_table_alter_column_set_not_null.go",
1314
"alter_table_alter_column_set_on_update.go",

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ var supportedAlterTableStatements = map[reflect.Type]supportedAlterTableCommand{
4343
reflect.TypeOf((*tree.AlterTableDropNotNull)(nil)): {fn: alterTableDropNotNull, on: true, checks: isV253Active},
4444
reflect.TypeOf((*tree.AlterTableSetOnUpdate)(nil)): {fn: alterTableSetOnUpdate, on: true, checks: isV254Active},
4545
reflect.TypeOf((*tree.AlterTableRenameColumn)(nil)): {fn: alterTableRenameColumn, on: true, checks: isV254Active},
46+
reflect.TypeOf((*tree.AlterTableDropStored)(nil)): {fn: alterTableDropStored, on: true, checks: isV261Active},
4647
}
4748

4849
func init() {
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
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+
func alterTableDropStored(
18+
b BuildCtx,
19+
tn *tree.TableName,
20+
tbl *scpb.Table,
21+
stmt tree.Statement,
22+
t *tree.AlterTableDropStored,
23+
) {
24+
alterColumnPreChecks(b, tn, tbl, t.Column)
25+
colElems := b.ResolveColumn(tbl.TableID, t.Column, ResolveParams{
26+
RequiredPrivilege: privilege.CREATE,
27+
})
28+
colElem := colElems.FilterColumn().MustGetOneElement()
29+
columnID := colElem.ColumnID
30+
// Block alters on system columns.
31+
panicIfSystemColumn(colElem, t.Column)
32+
// Ensure that the column is stored (not virtual)
33+
panicIfVirtualColumn(b, tbl.TableID, columnID, t.Column)
34+
// Retrieve the compute expression element
35+
exprElem := retrieveColumnComputeExpressionElem(b, tbl.TableID, columnID)
36+
// Ensure that the column is computed
37+
if exprElem == nil {
38+
panic(pgerror.Newf(
39+
pgcode.InvalidColumnDefinition,
40+
"column %q is not a computed column",
41+
tree.ErrString(&t.Column)))
42+
}
43+
b.Drop(exprElem)
44+
}
45+
46+
// panicIfVirtualColumn blocks operation if the column is virtual.
47+
func panicIfVirtualColumn(
48+
b BuildCtx, tableID catid.DescID, columnID catid.ColumnID, columnName tree.Name,
49+
) {
50+
colTypeEl := mustRetrieveColumnTypeElem(b, tableID, columnID)
51+
if colTypeEl.IsVirtual {
52+
panic(pgerror.Newf(
53+
pgcode.InvalidColumnDefinition,
54+
"column %q is not a stored computed column",
55+
tree.ErrString(&columnName)))
56+
}
57+
}
58+
59+
// retrieveColumnComputeExpressionElem returns the compute expression
60+
// element of the column. Returns nil if no expression exists and for
61+
// older versions that store the expression as part of the ColumnType
62+
func retrieveColumnComputeExpressionElem(
63+
b BuildCtx, tableID catid.DescID, columnID catid.ColumnID,
64+
) (expr *scpb.ColumnComputeExpression) {
65+
return b.QueryByID(tableID).FilterColumnComputeExpression().Filter(func(
66+
_ scpb.Status, _ scpb.TargetStatus, e *scpb.ColumnComputeExpression) bool {
67+
return e.ColumnID == columnID
68+
}).MustGetZeroOrOneElement()
69+
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,3 +227,8 @@ var isV253Active = func(_ tree.NodeFormatter, _ sessiondatapb.NewSchemaChangerMo
227227
var isV254Active = func(_ tree.NodeFormatter, _ sessiondatapb.NewSchemaChangerMode, activeVersion clusterversion.ClusterVersion) bool {
228228
return activeVersion.IsActive(clusterversion.V25_4)
229229
}
230+
231+
var isV261Active = func(_ tree.NodeFormatter, _ sessiondatapb.NewSchemaChangerMode, activeVersion clusterversion.ClusterVersion) bool {
232+
// change the cluster version once V26_1 is created
233+
return activeVersion.IsActive(clusterversion.V25_4)
234+
}

pkg/sql/schemachanger/scbuild/testdata/unimplemented_alter_table

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,6 @@ CREATE TABLE defaultdb.foo (
1212
);
1313
----
1414

15-
unimplemented
16-
ALTER TABLE defaultdb.foo ALTER COLUMN i DROP STORED
17-
----
18-
1915
unimplemented
2016
ALTER TABLE defaultdb.foo RENAME CONSTRAINT foobar TO baz
2117
----

pkg/sql/schemachanger/sctest_generated_test.go

Lines changed: 42 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
setup
2+
CREATE TABLE t (i INT PRIMARY KEY, j INT AS (i + 1) STORED);
3+
----
4+
5+
test
6+
ALTER TABLE t ALTER COLUMN j DROP STORED
7+
----
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/* setup */
2+
CREATE TABLE t (i INT PRIMARY KEY, j INT AS (i + 1) STORED);
3+
4+
/* test */
5+
EXPLAIN (DDL) ALTER TABLE t ALTER COLUMN j DROP STORED;
6+
----
7+
Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t› ALTER COLUMN ‹j› DROP STORED;
8+
├── StatementPhase
9+
│ └── Stage 1 of 1 in StatementPhase
10+
│ ├── 1 element transitioning toward TRANSIENT_PUBLIC
11+
│ │ └── PUBLIC → ABSENT TableSchemaLocked:{DescID: 104 (t)}
12+
│ ├── 1 element transitioning toward ABSENT
13+
│ │ └── PUBLIC → ABSENT ColumnComputeExpression:{DescID: 104 (t), ColumnID: 2 (j), ReferencedColumnIDs: [1], Usage: REGULAR}
14+
│ └── 2 Mutation operations
15+
│ ├── SetTableSchemaLocked {"TableID":104}
16+
│ └── RemoveColumnComputeExpression {"ColumnID":2,"TableID":104}
17+
├── PreCommitPhase
18+
│ ├── Stage 1 of 2 in PreCommitPhase
19+
│ │ ├── 1 element transitioning toward TRANSIENT_PUBLIC
20+
│ │ │ └── ABSENT → PUBLIC TableSchemaLocked:{DescID: 104 (t)}
21+
│ │ ├── 1 element transitioning toward ABSENT
22+
│ │ │ └── ABSENT → PUBLIC ColumnComputeExpression:{DescID: 104 (t), ColumnID: 2 (j), ReferencedColumnIDs: [1], Usage: REGULAR}
23+
│ │ └── 1 Mutation operation
24+
│ │ └── UndoAllInTxnImmediateMutationOpSideEffects
25+
│ └── Stage 2 of 2 in PreCommitPhase
26+
│ ├── 1 element transitioning toward TRANSIENT_PUBLIC
27+
│ │ └── PUBLIC → ABSENT TableSchemaLocked:{DescID: 104 (t)}
28+
│ ├── 1 element transitioning toward ABSENT
29+
│ │ └── PUBLIC → ABSENT ColumnComputeExpression:{DescID: 104 (t), ColumnID: 2 (j), ReferencedColumnIDs: [1], Usage: REGULAR}
30+
│ └── 4 Mutation operations
31+
│ ├── SetTableSchemaLocked {"TableID":104}
32+
│ ├── RemoveColumnComputeExpression {"ColumnID":2,"TableID":104}
33+
│ ├── SetJobStateOnDescriptor {"DescriptorID":104,"Initialize":true}
34+
│ └── CreateSchemaChangerJob {"RunningStatus":"Pending: Updatin..."}
35+
└── PostCommitPhase
36+
└── Stage 1 of 1 in PostCommitPhase
37+
├── 1 element transitioning toward TRANSIENT_PUBLIC
38+
│ └── ABSENT → TRANSIENT_PUBLIC TableSchemaLocked:{DescID: 104 (t)}
39+
└── 3 Mutation operations
40+
├── SetTableSchemaLocked {"Locked":true,"TableID":104}
41+
├── RemoveJobStateFromDescriptor {"DescriptorID":104}
42+
└── UpdateSchemaChangerJob {"IsNonCancelable":true,"RunningStatus":"all stages compl..."}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
/* setup */
2+
CREATE TABLE t (i INT PRIMARY KEY, j INT AS (i + 1) STORED);
3+
4+
/* test */
5+
EXPLAIN (DDL, SHAPE) ALTER TABLE t ALTER COLUMN j DROP STORED;
6+
----
7+
Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t› ALTER COLUMN ‹j› DROP STORED;
8+
└── execute 2 system table mutations transactions

0 commit comments

Comments
 (0)