Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion pkg/sql/schemachanger/scdecomp/decomp.go
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,6 @@ func (w *walkCtx) walkColumn(tbl catalog.TableDescriptor, col catalog.Column) {
column := &scpb.Column{
TableID: tbl.GetID(),
ColumnID: col.GetID(),
IsHidden: col.IsHidden(),
IsInaccessible: col.IsInaccessible(),
GeneratedAsIdentityType: col.GetGeneratedAsIdentityType(),
GeneratedAsIdentitySequenceOption: col.GetGeneratedAsIdentitySequenceOptionStr(),
Expand Down Expand Up @@ -591,6 +590,18 @@ func (w *walkCtx) walkColumn(tbl catalog.TableDescriptor, col catalog.Column) {
}
}
w.ev(scpb.Status_PUBLIC, columnType)

if col.IsHidden() {
if columnType.ElementCreationMetadata.In_26_1OrLater {
w.ev(scpb.Status_PUBLIC, &scpb.ColumnHidden{
TableID: tbl.GetID(),
ColumnID: col.GetID(),
})
} else {
column.IsHidden = true
}
}

}
if !col.IsNullable() {
w.ev(scpb.Status_PUBLIC, &scpb.ColumnNotNull{
Expand Down Expand Up @@ -633,6 +644,7 @@ func (w *walkCtx) walkColumn(tbl catalog.TableDescriptor, col catalog.Column) {
ColumnID: col.GetID(),
})
})

}

func (w *walkCtx) walkIndex(tbl catalog.TableDescriptor, idx catalog.Index) {
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/schemachanger/scdecomp/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,5 +142,6 @@ func NewElementCreationMetadata(
return &scpb.ElementCreationMetadata{
In_23_1OrLater: true,
In_24_3OrLater: true,
In_26_1OrLater: clusterVersion.IsActive(clusterversion.V25_4), // FIXME: need 26.1
}
}
15 changes: 14 additions & 1 deletion pkg/sql/schemachanger/scpb/elements.proto
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
//
// Use of this software is governed by the CockroachDB Software License
// included in the /LICENSE file.
//
// This file is used by `element_generator.go` to build `elements_generated.go`.
// It's sensitive to formatting; tools such as `buf format` aren't recommended.

syntax = "proto3";
package cockroach.sql.schemachanger.scpb;
Expand Down Expand Up @@ -109,6 +112,7 @@ message ElementProto {
ColumnComment column_comment = 35 [(gogoproto.moretags) = "parent:\"Column\""];
ColumnNotNull column_not_null = 36 [(gogoproto.moretags) = "parent:\"Column\""];
ColumnComputeExpression column_compute_expression = 190 [(gogoproto.moretags) = "parent:\"Column\""];
ColumnHidden column_hidden = 191 [(gogoproto.moretags) = "parent:\"Column\""];

// Sequence elements.
SequenceOption sequence_option = 37 [(gogoproto.moretags) = "parent:\"Sequence\""];
Expand Down Expand Up @@ -217,7 +221,9 @@ message Expression {
message Column {
uint32 table_id = 1 [(gogoproto.customname) = "TableID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"];
uint32 column_id = 2 [(gogoproto.customname) = "ColumnID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.ColumnID"];
bool is_hidden = 3;
// Deprecated.
// The hidden attribute is now handled as a separate element (see ColumnHidden).
bool is_hidden = 3 [deprecated = true];
bool is_inaccessible = 4;
uint32 generated_as_identity_type = 5 [(gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb.GeneratedAsIdentityType"];
string generated_as_identity_sequence_option = 6;
Expand Down Expand Up @@ -274,6 +280,12 @@ message ColumnComputeExpression {
Usage usage = 5;
}

message ColumnHidden {
uint32 table_id = 1 [(gogoproto.customname) = "TableID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"];
uint32 column_id = 2 [(gogoproto.customname) = "ColumnID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.ColumnID"];
}


message ColumnFamily {
uint32 table_id = 1 [(gogoproto.customname) = "TableID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"];
uint32 family_id = 2 [(gogoproto.customname) = "FamilyID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.FamilyID"];
Expand Down Expand Up @@ -1013,4 +1025,5 @@ message FunctionSecurity {
message ElementCreationMetadata {
bool in_23_1_or_later = 1;
bool in_24_3_or_later = 2;
bool in_26_1_or_later = 3;
}
41 changes: 41 additions & 0 deletions pkg/sql/schemachanger/scpb/elements_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 19 additions & 2 deletions pkg/sql/schemachanger/scpb/migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func migrateDeprecatedFields(
}
}

// Migrate ComputeExpr field to separate ColumnComputeExpression target.
// Migrate ComputeExpr field to separate ColumnComputeExpression target.
if columnType := target.GetColumnType(); columnType != nil {
if columnType.ComputeExpr != nil {
newTarget := MakeTarget(
Expand All @@ -75,7 +75,24 @@ func migrateDeprecatedFields(
migrated = true
}
}
return

if column := target.GetColumn(); column != nil {
if column.IsHidden && version.IsActive(clusterversion.V25_4) { // FIXME: need clusterversion.V26_1 from release engineering
newTarget := MakeTarget(
AsTargetStatus(target.TargetStatus),
&ColumnHidden{
TableID: column.TableID,
ColumnID: column.ColumnID,
},
&target.Metadata,
)
newTargets = append(newTargets, newTarget)
column.IsHidden = false
migrated = true
}
}

return migrated, newTargets
}

// migrateTargetElement migrates an individual target at a given index.
Expand Down
37 changes: 37 additions & 0 deletions pkg/sql/schemachanger/scpb/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,3 +137,40 @@ func TestDeprecatedTriggerDeps(t *testing.T) {
require.Equal(t, catid.DescID(112), triggerDeps.UsesRelations[0].ID)
require.Equal(t, catid.DescID(113), triggerDeps.UsesRelations[1].ID)
}

func TestDeprecatedColumnHiddenMigration(t *testing.T) {
state := DescriptorState{
Targets: []Target{
MakeTarget(ToPublic,
&Column{
TableID: 100,
ColumnID: 105,
IsHidden: true,
},
nil,
),
},
CurrentStatuses: []Status{Status_PUBLIC},
TargetRanks: []uint32{1},
}
migrationOccurred := MigrateDescriptorState(
clusterversion.ClusterVersion{Version: clusterversion.Latest.Version()},
1,
&state,
)
require.True(t, migrationOccurred)
require.Len(t, state.CurrentStatuses, 2)
require.Len(t, state.Targets, 2)

column := state.Targets[0].GetColumn()
require.NotNil(t, column)
require.False(t, column.IsHidden)

columnHidden := state.Targets[1].GetColumnHidden()
require.NotNil(t, columnHidden)

require.Equal(t, column.TableID, columnHidden.TableID)
require.Equal(t, column.TableID, columnHidden.TableID)
require.Equal(t, state.CurrentStatuses[1], Status_PUBLIC)
require.Equal(t, state.TargetRanks[1], uint32(2))
}
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ func isColumnDependent(e scpb.Element) bool {
switch e.(type) {
case *scpb.ColumnType, *scpb.ColumnNotNull:
return true
case *scpb.ColumnName, *scpb.ColumnComment, *scpb.IndexColumn:
case *scpb.ColumnName, *scpb.ColumnComment, *scpb.IndexColumn, *scpb.ColumnHidden:
return true
}
return isColumnTypeDependent(e)
Expand All @@ -272,6 +272,7 @@ func isColumnNotNull(e scpb.Element) bool {
}
return false
}

func isColumnTypeDependent(e scpb.Element) bool {
switch e.(type) {
case *scpb.SequenceOwner, *scpb.ColumnDefaultExpression, *scpb.ColumnOnUpdateExpression, *scpb.ColumnComputeExpression:
Expand Down
8 changes: 7 additions & 1 deletion pkg/sql/schemachanger/screl/attr.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,9 @@ const (

var t = reflect.TypeOf

// elementSchemaOptions maps attributes to the elements' fields.
var elementSchemaOptions = []rel.SchemaOption{
// We need this `Element` attribute to be of type `protoulti.Message`
// We need this `Element` attribute to be of type `protoutil.Message`
// interface and better have it as the first in the schema option list. This
// is because the schema needs to know a type of each attribute, and it
// creates a mapping between attribute and the type. If you're trying to add a
Expand All @@ -136,6 +137,7 @@ var elementSchemaOptions = []rel.SchemaOption{
// concrete type underneath an interface value, so we won't have a problem
// evaluating field values within a concrete Element struct.
rel.AttrType(Element, t((*protoutil.Message)(nil)).Elem()),

// Top-level elements.
rel.EntityMapping(t((*scpb.Database)(nil)),
rel.EntityAttr(DescID, "DatabaseID"),
Expand Down Expand Up @@ -322,6 +324,10 @@ var elementSchemaOptions = []rel.SchemaOption{
rel.EntityAttr(ColumnID, "ColumnID"),
rel.EntityAttr(IndexID, "IndexIDForValidation"),
),
rel.EntityMapping(t((*scpb.ColumnHidden)(nil)),
rel.EntityAttr(DescID, "TableID"),
rel.EntityAttr(ColumnID, "ColumnID"),
),
// Index elements.
rel.EntityMapping(t((*scpb.IndexName)(nil)),
rel.EntityAttr(DescID, "TableID"),
Expand Down
2 changes: 2 additions & 0 deletions pkg/sql/schemachanger/screl/scalars.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ func VersionSupportsElementUse(el scpb.Element, version clusterversion.ClusterVe
return version.IsActive(clusterversion.V25_2)
case *scpb.TableLocalityRegionalByRowUsingConstraint:
return version.IsActive(clusterversion.V25_3)
case *scpb.ColumnHidden:
return version.IsActive(clusterversion.V25_4) // FIXME: need clusterversion.V26_1 from release engineering
default:
panic(errors.AssertionFailedf("unknown element %T", el))
}
Expand Down
Loading