Skip to content

Commit

Permalink
sql/schemachanger: copy ALTER TABLE .. SET TYPE pre-checks from legac…
Browse files Browse the repository at this point in the history
…y to DSC

This duplicates various pre-checks we have in the legacy schema
changer's ALTER TABLE .. SET TYPE command to the declarative schema
changer, which uses states from various schema changer elements.

Additionally, it introduces more tests for validation-only type
conversions. These conversions don't require data to be rewritten;
instead, they validate existing rows against the new type. For example,
changing from CHAR(20) to CHAR(10) requires ensuring each row is 10
characters or fewer.

The actual implementation of validation-only type conversions in the DSC
will be handled in a subsequent PR.

Epic: CRDB-25314
Informs: #126516
Release note: None
  • Loading branch information
spilchen committed Jul 26, 2024
1 parent ac0b25c commit 359e6ae
Show file tree
Hide file tree
Showing 7 changed files with 470 additions and 68 deletions.
27 changes: 9 additions & 18 deletions pkg/sql/alter_column_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,12 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sem/cast"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sem/volatility"
"github.com/cockroachdb/cockroach/pkg/sql/sqlerrors"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
"github.com/cockroachdb/errors"
)

var colInIndexNotSupportedErr = unimplemented.NewWithIssuef(
47636, "ALTER COLUMN TYPE requiring rewrite of on-disk "+
"data is currently not supported for columns that are part of an index")

var colOwnsSequenceNotSupportedErr = unimplemented.NewWithIssuef(
48244, "ALTER COLUMN TYPE for a column that owns a sequence "+
"is currently not supported")

var colWithConstraintNotSupportedErr = unimplemented.NewWithIssuef(
48288, "ALTER COLUMN TYPE for a column that has a constraint "+
"is currently not supported")

// AlterColTypeInTxnNotSupportedErr is returned when an ALTER COLUMN TYPE
// is tried in an explicit transaction.
var AlterColTypeInTxnNotSupportedErr = unimplemented.NewWithIssuef(
Expand Down Expand Up @@ -177,34 +166,36 @@ func alterColumnTypeGeneral(

// Disallow ALTER COLUMN TYPE general for columns that own sequences.
if col.NumOwnsSequences() != 0 {
return colOwnsSequenceNotSupportedErr
return sqlerrors.NewAlterColumnTypeColOwnsSequenceNotSupportedErr()
}

// Disallow ALTER COLUMN TYPE general for columns that have a check
// constraint.
for _, ck := range tableDesc.EnforcedCheckConstraints() {
if ck.CollectReferencedColumnIDs().Contains(col.GetID()) {
return colWithConstraintNotSupportedErr
return sqlerrors.NewAlterColumnTypeColWithConstraintNotSupportedErr()
}
}

// Disallow ALTER COLUMN TYPE general for columns that have a
// UNIQUE WITHOUT INDEX constraint.
for _, uc := range tableDesc.UniqueConstraintsWithoutIndex() {
if uc.CollectKeyColumnIDs().Contains(col.GetID()) {
return colWithConstraintNotSupportedErr
return sqlerrors.NewAlterColumnTypeColWithConstraintNotSupportedErr()
}
}

// Disallow ALTER COLUMN TYPE general for columns that have a foreign key
// constraint.
for _, fk := range tableDesc.OutboundForeignKeys() {
if fk.CollectOriginColumnIDs().Contains(col.GetID()) {
return colWithConstraintNotSupportedErr
return sqlerrors.NewAlterColumnTypeColWithConstraintNotSupportedErr()
}
}
for _, fk := range tableDesc.InboundForeignKeys() {
if fk.GetReferencedTableID() == tableDesc.GetID() &&
fk.CollectReferencedColumnIDs().Contains(col.GetID()) {
return colWithConstraintNotSupportedErr
return sqlerrors.NewAlterColumnTypeColWithConstraintNotSupportedErr()
}
}

Expand All @@ -214,7 +205,7 @@ func alterColumnTypeGeneral(
if idx.CollectKeyColumnIDs().Contains(col.GetID()) ||
idx.CollectKeySuffixColumnIDs().Contains(col.GetID()) ||
idx.CollectSecondaryStoredColumnIDs().Contains(col.GetID()) {
return colInIndexNotSupportedErr
return sqlerrors.NewAlterColumnTypeColInIndexNotSupportedErr()
}
}

Expand Down
Loading

0 comments on commit 359e6ae

Please sign in to comment.