Skip to content

Commit 9ed3d60

Browse files
committed
sql: implement hidden column element
This changes moves `ALTER COLUMN SET [NOT] VISIBLE` statements to the declarative schema changer. Epic: CRDB-31283 Fixes: #139605 Release note (sql change): The `ALTER TABLE t ALTER COLUMN c SET [NOT] VISIBLE` statements now use the declarative schema changer.
1 parent a2134d8 commit 9ed3d60

17 files changed

+652
-7
lines changed

pkg/ccl/schemachangerccl/sctestbackupccl/backup_base_generated_test.go

Lines changed: 56 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/cmd/roachtest/tests/mixed_version_job_compatibility_in_declarative_schema_changer.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,12 @@ func executeSupportedDDLs(
123123
`ALTER TABLE testdb.testsc.t2 RENAME COLUMN k_renamed TO k`,
124124
}
125125

126+
// DDLs supported in V26_1.
127+
v261DDLs := []string{
128+
`ALTER TABLE testdb.testsc.t2 ALTER COLUMN j SET NOT VISIBLE`,
129+
`ALTER TABLE testdb.testsc.t2 ALTER COLUMN j SET VISIBLE`,
130+
}
131+
126132
// Used to clean up our CREATE-d elements after we are done with them.
127133
cleanup := []string{
128134
`DROP INDEX testdb.testsc.t@idx`,
@@ -155,6 +161,13 @@ func executeSupportedDDLs(
155161
}
156162
}
157163
}
164+
if clusterVersion.AtLeast(clusterversion.V26_1.Version()) {
165+
for _, ddl := range v261DDLs {
166+
if err := helper.ExecWithGateway(r, nodes, ddl); err != nil {
167+
return err
168+
}
169+
}
170+
}
158171

159172
for _, ddl := range cleanup {
160173
if err := helper.ExecWithGateway(r, nodes, ddl); err != nil {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ go_library(
1414
"alter_table_alter_column_set_identity.go",
1515
"alter_table_alter_column_set_not_null.go",
1616
"alter_table_alter_column_set_on_update.go",
17+
"alter_table_alter_column_set_visible.go",
1718
"alter_table_alter_column_type.go",
1819
"alter_table_alter_primary_key.go",
1920
"alter_table_drop_column.go",

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ import (
2626

2727
type supportedAlterTableCommand = supportedStatement
2828

29-
// supportedAlterTableStatements tracks alter table operations fully supported by
30-
// declarative schema changer. Operations marked as non-fully supported can
31-
// only be with the use_declarative_schema_changer session variable.
29+
// supportedAlterTableStatements tracks alter table operations fully supported
30+
// by the declarative schema changer. Operations marked as non-fully supported
31+
// can only be with the use_declarative_schema_changer session variable.
3232
var supportedAlterTableStatements = map[reflect.Type]supportedAlterTableCommand{
3333
reflect.TypeOf((*tree.AlterTableAddColumn)(nil)): {fn: alterTableAddColumn, on: true, checks: nil},
3434
reflect.TypeOf((*tree.AlterTableDropColumn)(nil)): {fn: alterTableDropColumn, on: true, checks: nil},
@@ -47,6 +47,7 @@ var supportedAlterTableStatements = map[reflect.Type]supportedAlterTableCommand{
4747
reflect.TypeOf((*tree.AlterTableRenameConstraint)(nil)): {fn: alterTableRenameConstraint, on: true, checks: isV261Active},
4848
reflect.TypeOf((*tree.AlterTableSetIdentity)(nil)): {fn: alterTableSetIdentity, on: true, checks: isV261Active},
4949
reflect.TypeOf((*tree.AlterTableAddIdentity)(nil)): {fn: alterTableAddIdentity, on: true, checks: isV261Active},
50+
reflect.TypeOf((*tree.AlterTableSetVisible)(nil)): {fn: alterTableAlterColumnSetVisible, on: true, checks: isV261Active},
5051
}
5152

5253
func init() {
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
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/schemachanger/scpb"
10+
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
11+
)
12+
13+
func alterTableAlterColumnSetVisible(
14+
b BuildCtx,
15+
tn *tree.TableName,
16+
tbl *scpb.Table,
17+
stmt tree.Statement,
18+
t *tree.AlterTableSetVisible,
19+
) {
20+
alterColumnPreChecks(b, tn, tbl, t.Column)
21+
columnID := getColumnIDFromColumnName(b, tbl.TableID, t.Column, true /* required */)
22+
// Block alters on system columns.
23+
panicIfSystemColumn(mustRetrieveColumnElem(b, tbl.TableID, columnID), t.Column)
24+
25+
columnHidden := b.QueryByID(tbl.TableID).FilterColumnHidden().Filter(func(current scpb.Status, target scpb.TargetStatus, e *scpb.ColumnHidden) bool {
26+
return e.ColumnID == columnID
27+
}).MustGetZeroOrOneElement()
28+
29+
if columnHidden != nil && t.Visible {
30+
b.Drop(columnHidden)
31+
} else if columnHidden == nil && !t.Visible {
32+
b.Add(&scpb.ColumnHidden{TableID: tbl.TableID, ColumnID: columnID})
33+
}
34+
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
setup
2+
CREATE TABLE defaultdb.foo (i INT PRIMARY KEY, a INT, b INT NOT VISIBLE);
3+
----
4+
5+
build
6+
ALTER TABLE foo ALTER COLUMN a SET VISIBLE
7+
----
8+
9+
build
10+
ALTER TABLE foo ALTER COLUMN b SET VISIBLE
11+
----
12+
- [[ColumnHidden:{DescID: 104, ColumnID: 3}, ABSENT], PUBLIC]
13+
{columnId: 3, tableId: 104}
14+
- [[IndexData:{DescID: 104, IndexID: 1}, PUBLIC], PUBLIC]
15+
{indexId: 1, tableId: 104}
16+
- [[TableData:{DescID: 104, ReferencedDescID: 100}, PUBLIC], PUBLIC]
17+
{databaseId: 100, tableId: 104}
18+
19+
build
20+
ALTER TABLE foo ALTER COLUMN a SET NOT VISIBLE
21+
----
22+
- [[IndexData:{DescID: 104, IndexID: 1}, PUBLIC], PUBLIC]
23+
{indexId: 1, tableId: 104}
24+
- [[TableData:{DescID: 104, ReferencedDescID: 100}, PUBLIC], PUBLIC]
25+
{databaseId: 100, tableId: 104}
26+
- [[ColumnHidden:{DescID: 104, ColumnID: 2}, PUBLIC], ABSENT]
27+
{columnId: 2, tableId: 104}
28+
29+
build
30+
ALTER TABLE foo ALTER COLUMN a SET NOT VISIBLE
31+
----
32+
- [[IndexData:{DescID: 104, IndexID: 1}, PUBLIC], PUBLIC]
33+
{indexId: 1, tableId: 104}
34+
- [[TableData:{DescID: 104, ReferencedDescID: 100}, PUBLIC], PUBLIC]
35+
{databaseId: 100, tableId: 104}
36+
- [[ColumnHidden:{DescID: 104, ColumnID: 2}, PUBLIC], ABSENT]
37+
{columnId: 2, tableId: 104}

pkg/sql/schemachanger/scdecomp/decomp.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -662,7 +662,6 @@ func (w *walkCtx) walkColumn(tbl catalog.TableDescriptor, col catalog.Column) {
662662
ColumnID: col.GetID(),
663663
})
664664
})
665-
666665
}
667666

668667
func (w *walkCtx) walkIndex(tbl catalog.TableDescriptor, idx catalog.Index) {

pkg/sql/schemachanger/screl/scalars.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,7 @@ func VersionSupportsElementUse(el scpb.Element, version clusterversion.ClusterVe
136136
return version.IsActive(clusterversion.V25_2)
137137
case *scpb.TableLocalityRegionalByRowUsingConstraint:
138138
return version.IsActive(clusterversion.V25_3)
139-
case *scpb.ColumnGeneratedAsIdentity:
140-
return version.IsActive(clusterversion.V26_1)
141-
case *scpb.ColumnHidden:
139+
case *scpb.ColumnGeneratedAsIdentity, *scpb.ColumnHidden:
142140
return version.IsActive(clusterversion.V26_1)
143141
default:
144142
panic(errors.AssertionFailedf("unknown element %T", el))

0 commit comments

Comments
 (0)