Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

sql/schemachanger: support validation only ALTER COLUMN .. SET TYPE changes in the declarative schema changer #127516

Closed
spilchen opened this issue Jul 19, 2024 · 0 comments · Fixed by #127823
Closed

sql/schemachanger: support validation only ALTER COLUMN .. SET TYPE changes in the declarative schema changer #127516

spilchen opened this issue Jul 19, 2024 · 0 comments · Fixed by #127823
Assignees
@spilchen
Labels
A-schema-changer-impl Related to the implementation of the new schema changer C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@spilchen
Copy link
Contributor

spilchen commented Jul 19, 2024 •
edited by cockroach-jira-scripts
Loading

This is a continuation of #126143. It will continue to move more ALTER COLUMN ... TYPE scenarios into the declarative schema changer. This issue deals specifically with changes that require validation only. This is for type changes that don't require a rewrite, just range checking. For instance, if changing the type from a BIGINT -> SMALLINT, we need to ensure that all of the existing values fit within the SMALLINT range.

Epic: CRDB-25314

Jira issue: CRDB-40465
@spilchen spilchen added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) A-schema-changer-impl Related to the implementation of the new schema changer labels Jul 19, 2024
@spilchen spilchen self-assigned this Jul 19, 2024
@spilchen spilchen mentioned this issue Jul 26, 2024
Merged
craig bot pushed a commit that referenced this issue Jul 26, 2024
@sudomateo @spilchen
Merge #126601 #127533
b97a858 
126601: drtprod: emit audit logs for drt-chaos r=sudomateo a=sudomateo

This patch updates the Fluent Bit configuration for DRT clusters to emit audit logs. This is only configured for the `drt-chaos` cluster for now, but can easily be expanded to other clusters in the future.

This patch also updates the way Fluent Bit sets the hostname on logs since Fluent Bit has not cut a release that contains the fix for fluent/fluent-bit#8971.

Finally, this patch also fixes an issue with the Datadog agent configuration removal throwing a shell error.

Epic: CLOUDOPS-9609

Release note: None


127533: sql/schemachanger: copy ALTER TABLE .. SET TYPE validation checks from legacy to DSC r=spilchen a=spilchen

This duplicates various validation checks from the legacy schema changer's ALTER TABLE .. SET TYPE command for 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: #127516
Release note: None

Co-authored-by: Matthew Sanabria <[email protected]>
Co-authored-by: Matt Spilchen <[email protected]>
@spilchen spilchen mentioned this issue Jul 29, 2024
Merged
spilchen added a commit to spilchen/cockroach that referenced this issue Jul 30, 2024
@spilchen
sql/schemachanger: Validate ALTER TABLE .. TYPE in the DSC
6ab36d4 
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
spilchen added a commit to spilchen/cockroach that referenced this issue Jul 31, 2024
@spilchen
sql/schemachanger: Validate ALTER TABLE .. TYPE in the DSC
d4c2360 
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
spilchen added a commit to spilchen/cockroach that referenced this issue Aug 1, 2024
@spilchen
sql/schemachanger: Validate ALTER TABLE .. TYPE in the DSC
cb8cf09 
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
craig bot pushed a commit that referenced this issue Aug 2, 2024
@spilchen
Merge #127823
b610202 
127823: sql/schemachanger: Validate ALTER TABLE .. TYPE in the DSC r=fqazi,annrpom a=spilchen

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: #127516
Release note: None

Co-authored-by: Matt Spilchen <[email protected]>
@craig craig bot closed this as completed in c22dd0a Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@spilchen spilchen

Labels
A-schema-changer-impl Related to the implementation of the new schema changer C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
No open projects
SQL Foundations
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sql/schemachanger: Validate ALTER TABLE .. TYPE in the DSC spilchen/cockroach
1 participant
@spilchen