Skip to content

Commit

Permalink
sql/schemachanger: Validate ALTER TABLE .. TYPE in the DSC
Browse files Browse the repository at this point in the history 
When altering the data type of a column, some changes only require
validating the existing data. For example, when changing from `CHAR(20)`
to `CHAR(10)`, we don't need to rewrite the data; we only need to ensure
all existing data fits within `CHAR(10)`. This change implements that
validation logic in the declarative schema changer.

The validation logic uses a temporary check constraint. This constraint
is added transiently so that it never transitions to PUBLIC and is
automatically removed. The check constraint casts the column to the new
type and then back to the old type. If the value matches its original
value after these casts, the row is deemed valid.

To add a schema changer test, I disabled the
`enable_experimental_alter_column_type_general` setting for validation.
It remains enabled for column type alterations requiring a rewrite and
for validation-only changes in the legacy schema changer.

I updated the dependency rules to allow the check constraint to be added
transiently. I think this is fine even for older releases because I'm not
introducing new elements or attributes for the check constraint; I'm
just allowing a state transition to transient.

When validation occurs, it will look something like this:

```
[email protected]:26257/demoapp/movr> ALTER TABLE t1 ALTER COLUMN c1 SET
DATA TYPE CHAR(3);
ERROR: validation of CHECK "CAST(CAST(c1 AS CHAR(3)) AS CHAR(20)) = c1"
failed on row: c1='123456789', rowid=990187907647504385
SQLSTATE: 23514
```

Epic: CRDB-25314
Closes: cockroachdb#127516
Release note: None
  • Loading branch information
@spilchen
spilchen committed Jul 31, 2024
1 parent c7d1cee commit d4c2360
Show file tree
Hide file tree
Showing 18 changed files with 1,992 additions and 76 deletions.
4 changes: 4 additions & 0 deletions pkg/ccl/changefeedccl/cdcevent/event_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,10 @@ func TestEventColumnOrderingWithSchemaChanges(t *testing.T) {
sqlDB := sqlutils.MakeSQLRunner(db)
// Use alter column type to force column reordering.
sqlDB.Exec(t, `SET enable_experimental_alter_column_type_general = true`)
// TODO(spilchen): column reordering implies that we are doing a backfill.
// Use the legacy schema changer for now, as the declarative schema changer
// cannot do backfill yet. This will be fixed in issue #127014.
sqlDB.Exec(t, `SET use_declarative_schema_changer = off`)

type decodeExpectation struct {
expectUnwatchedErr bool
Expand Down
28 changes: 28 additions & 0 deletions pkg/ccl/schemachangerccl/backup_base_generated_test.go
View file

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

192 changes: 191 additions & 1 deletion pkg/cli/testdata/declarative-rules/deprules
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,69 @@ deprules
- descriptorIsDataNotBeingAdded-24.1($descID)
- joinTargetNode($prev, $prev-Target, $prev-Node)
- joinTargetNode($next, $next-Target, $next-Node)
- name: 'CheckConstraint transitions to ABSENT uphold 2-version invariant: TRANSIENT_ABSENT->ABSENT'
from: prev-Node
kind: PreviousTransactionPrecedence
to: next-Node
query:
- $prev[Type] = '*scpb.CheckConstraint'
- $next[Type] = '*scpb.CheckConstraint'
- $prev[DescID] = $descID
- $prev[Self] = $next
- $prev-Target[Self] = $next-Target
- $prev-Target[TargetStatus] = ABSENT
- $prev-Node[CurrentStatus] = TRANSIENT_ABSENT
- $next-Node[CurrentStatus] = ABSENT
- descriptorIsNotBeingDropped-24.1($prev)
- $descriptor-data[Type] = '*scpb.TableData'
- joinTargetNode($descriptor-data, $descriptor-data-Target, $descriptor-data-Node)
- $descriptor-data-Node[CurrentStatus] = PUBLIC
- $descriptor-data[DescID] = $descID
- descriptorIsDataNotBeingAdded-24.1($descID)
- joinTargetNode($prev, $prev-Target, $prev-Node)
- joinTargetNode($next, $next-Target, $next-Node)
- name: 'CheckConstraint transitions to ABSENT uphold 2-version invariant: TRANSIENT_VALIDATED->VALIDATED'
from: prev-Node
kind: PreviousTransactionPrecedence
to: next-Node
query:
- $prev[Type] = '*scpb.CheckConstraint'
- $next[Type] = '*scpb.CheckConstraint'
- $prev[DescID] = $descID
- $prev[Self] = $next
- $prev-Target[Self] = $next-Target
- $prev-Target[TargetStatus] = ABSENT
- $prev-Node[CurrentStatus] = TRANSIENT_VALIDATED
- $next-Node[CurrentStatus] = VALIDATED
- descriptorIsNotBeingDropped-24.1($prev)
- $descriptor-data[Type] = '*scpb.TableData'
- joinTargetNode($descriptor-data, $descriptor-data-Target, $descriptor-data-Node)
- $descriptor-data-Node[CurrentStatus] = PUBLIC
- $descriptor-data[DescID] = $descID
- descriptorIsDataNotBeingAdded-24.1($descID)
- joinTargetNode($prev, $prev-Target, $prev-Node)
- joinTargetNode($next, $next-Target, $next-Node)
- name: 'CheckConstraint transitions to ABSENT uphold 2-version invariant: TRANSIENT_WRITE_ONLY->VALIDATED'
from: prev-Node
kind: PreviousTransactionPrecedence
to: next-Node
query:
- $prev[Type] = '*scpb.CheckConstraint'
- $next[Type] = '*scpb.CheckConstraint'
- $prev[DescID] = $descID
- $prev[Self] = $next
- $prev-Target[Self] = $next-Target
- $prev-Target[TargetStatus] = ABSENT
- $prev-Node[CurrentStatus] = TRANSIENT_WRITE_ONLY
- $next-Node[CurrentStatus] = VALIDATED
- descriptorIsNotBeingDropped-24.1($prev)
- $descriptor-data[Type] = '*scpb.TableData'
- joinTargetNode($descriptor-data, $descriptor-data-Target, $descriptor-data-Node)
- $descriptor-data-Node[CurrentStatus] = PUBLIC
- $descriptor-data[DescID] = $descID
- descriptorIsDataNotBeingAdded-24.1($descID)
- joinTargetNode($prev, $prev-Target, $prev-Node)
- joinTargetNode($next, $next-Target, $next-Node)
- name: 'CheckConstraint transitions to ABSENT uphold 2-version invariant: VALIDATED->ABSENT'
from: prev-Node
kind: PreviousTransactionPrecedence
Expand All @@ -43,7 +106,7 @@ deprules
- $descriptor-data-Node[CurrentStatus] = PUBLIC
- $descriptor-data[DescID] = $descID
- descriptorIsDataNotBeingAdded-24.1($descID)
- nodeNotExistsWithStatusIn_WRITE_ONLY($prev-Target)
- nodeNotExistsWithStatusIn_TRANSIENT_VALIDATED_WRITE_ONLY_TRANSIENT_WRITE_ONLY($prev-Target)
- joinTargetNode($prev, $prev-Target, $prev-Node)
- joinTargetNode($next, $next-Target, $next-Node)
- name: 'CheckConstraint transitions to ABSENT uphold 2-version invariant: WRITE_ONLY->VALIDATED'
Expand Down Expand Up @@ -130,6 +193,133 @@ deprules
- descriptorIsDataNotBeingAdded-24.1($descID)
- joinTargetNode($prev, $prev-Target, $prev-Node)
- joinTargetNode($next, $next-Target, $next-Node)
- name: 'CheckConstraint transitions to TRANSIENT_ABSENT uphold 2-version invariant: ABSENT->WRITE_ONLY'
from: prev-Node
kind: PreviousTransactionPrecedence
to: next-Node
query:
- $prev[Type] = '*scpb.CheckConstraint'
- $next[Type] = '*scpb.CheckConstraint'
- $prev[DescID] = $descID
- $prev[Self] = $next
- $prev-Target[Self] = $next-Target
- $prev-Target[TargetStatus] = TRANSIENT_ABSENT
- $prev-Node[CurrentStatus] = ABSENT
- $next-Node[CurrentStatus] = WRITE_ONLY
- descriptorIsNotBeingDropped-24.1($prev)
- $descriptor-data[Type] = '*scpb.TableData'
- joinTargetNode($descriptor-data, $descriptor-data-Target, $descriptor-data-Node)
- $descriptor-data-Node[CurrentStatus] = PUBLIC
- $descriptor-data[DescID] = $descID
- descriptorIsDataNotBeingAdded-24.1($descID)
- joinTargetNode($prev, $prev-Target, $prev-Node)
- joinTargetNode($next, $next-Target, $next-Node)
- name: 'CheckConstraint transitions to TRANSIENT_ABSENT uphold 2-version invariant: PUBLIC->TRANSIENT_VALIDATED'
from: prev-Node
kind: PreviousTransactionPrecedence
to: next-Node
query:
- $prev[Type] = '*scpb.CheckConstraint'
- $next[Type] = '*scpb.CheckConstraint'
- $prev[DescID] = $descID
- $prev[Self] = $next
- $prev-Target[Self] = $next-Target
- $prev-Target[TargetStatus] = TRANSIENT_ABSENT
- $prev-Node[CurrentStatus] = PUBLIC
- $next-Node[CurrentStatus] = TRANSIENT_VALIDATED
- descriptorIsNotBeingDropped-24.1($prev)
- $descriptor-data[Type] = '*scpb.TableData'
- joinTargetNode($descriptor-data, $descriptor-data-Target, $descriptor-data-Node)
- $descriptor-data-Node[CurrentStatus] = PUBLIC
- $descriptor-data[DescID] = $descID
- descriptorIsDataNotBeingAdded-24.1($descID)
- joinTargetNode($prev, $prev-Target, $prev-Node)
- joinTargetNode($next, $next-Target, $next-Node)
- name: 'CheckConstraint transitions to TRANSIENT_ABSENT uphold 2-version invariant: TRANSIENT_VALIDATED->TRANSIENT_ABSENT'
from: prev-Node
kind: PreviousTransactionPrecedence
to: next-Node
query:
- $prev[Type] = '*scpb.CheckConstraint'
- $next[Type] = '*scpb.CheckConstraint'
- $prev[DescID] = $descID
- $prev[Self] = $next
- $prev-Target[Self] = $next-Target
- $prev-Target[TargetStatus] = TRANSIENT_ABSENT
- $prev-Node[CurrentStatus] = TRANSIENT_VALIDATED
- $next-Node[CurrentStatus] = TRANSIENT_ABSENT
- descriptorIsNotBeingDropped-24.1($prev)
- $descriptor-data[Type] = '*scpb.TableData'
- joinTargetNode($descriptor-data, $descriptor-data-Target, $descriptor-data-Node)
- $descriptor-data-Node[CurrentStatus] = PUBLIC
- $descriptor-data[DescID] = $descID
- descriptorIsDataNotBeingAdded-24.1($descID)
- nodeNotExistsWithStatusIn_TRANSIENT_WRITE_ONLY($prev-Target)
- joinTargetNode($prev, $prev-Target, $prev-Node)
- joinTargetNode($next, $next-Target, $next-Node)
- name: 'CheckConstraint transitions to TRANSIENT_ABSENT uphold 2-version invariant: TRANSIENT_WRITE_ONLY->TRANSIENT_VALIDATED'
from: prev-Node
kind: PreviousTransactionPrecedence
to: next-Node
query:
- $prev[Type] = '*scpb.CheckConstraint'
- $next[Type] = '*scpb.CheckConstraint'
- $prev[DescID] = $descID
- $prev[Self] = $next
- $prev-Target[Self] = $next-Target
- $prev-Target[TargetStatus] = TRANSIENT_ABSENT
- $prev-Node[CurrentStatus] = TRANSIENT_WRITE_ONLY
- $next-Node[CurrentStatus] = TRANSIENT_VALIDATED
- descriptorIsNotBeingDropped-24.1($prev)
- $descriptor-data[Type] = '*scpb.TableData'
- joinTargetNode($descriptor-data, $descriptor-data-Target, $descriptor-data-Node)
- $descriptor-data-Node[CurrentStatus] = PUBLIC
- $descriptor-data[DescID] = $descID
- descriptorIsDataNotBeingAdded-24.1($descID)
- joinTargetNode($prev, $prev-Target, $prev-Node)
- joinTargetNode($next, $next-Target, $next-Node)
- name: 'CheckConstraint transitions to TRANSIENT_ABSENT uphold 2-version invariant: VALIDATED->PUBLIC'
from: prev-Node
kind: PreviousTransactionPrecedence
to: next-Node
query:
- $prev[Type] = '*scpb.CheckConstraint'
- $next[Type] = '*scpb.CheckConstraint'
- $prev[DescID] = $descID
- $prev[Self] = $next
- $prev-Target[Self] = $next-Target
- $prev-Target[TargetStatus] = TRANSIENT_ABSENT
- $prev-Node[CurrentStatus] = VALIDATED
- $next-Node[CurrentStatus] = PUBLIC
- descriptorIsNotBeingDropped-24.1($prev)
- $descriptor-data[Type] = '*scpb.TableData'
- joinTargetNode($descriptor-data, $descriptor-data-Target, $descriptor-data-Node)
- $descriptor-data-Node[CurrentStatus] = PUBLIC
- $descriptor-data[DescID] = $descID
- descriptorIsDataNotBeingAdded-24.1($descID)
- joinTargetNode($prev, $prev-Target, $prev-Node)
- joinTargetNode($next, $next-Target, $next-Node)
- name: 'CheckConstraint transitions to TRANSIENT_ABSENT uphold 2-version invariant: WRITE_ONLY->VALIDATED'
from: prev-Node
kind: PreviousTransactionPrecedence
to: next-Node
query:
- $prev[Type] = '*scpb.CheckConstraint'
- $next[Type] = '*scpb.CheckConstraint'
- $prev[DescID] = $descID
- $prev[Self] = $next
- $prev-Target[Self] = $next-Target
- $prev-Target[TargetStatus] = TRANSIENT_ABSENT
- $prev-Node[CurrentStatus] = WRITE_ONLY
- $next-Node[CurrentStatus] = VALIDATED
- descriptorIsNotBeingDropped-24.1($prev)
- $descriptor-data[Type] = '*scpb.TableData'
- joinTargetNode($descriptor-data, $descriptor-data-Target, $descriptor-data-Node)
- $descriptor-data-Node[CurrentStatus] = PUBLIC
- $descriptor-data[DescID] = $descID
- descriptorIsDataNotBeingAdded-24.1($descID)
- joinTargetNode($prev, $prev-Target, $prev-Node)
- joinTargetNode($next, $next-Target, $next-Node)
- name: 'Column transitions to ABSENT uphold 2-version invariant: DELETE_ONLY->ABSENT'
from: prev-Node
kind: PreviousTransactionPrecedence
Expand Down
Loading

0 comments on commit d4c2360

Please sign in to comment.