Skip to content

Commit

Permalink
Merge cockroachdb#135315
Browse files Browse the repository at this point in the history 
135315: sql/schemachanger: Enable column type changes for virtual computed columns r=spilchen a=spilchen

This update introduces support for altering the column type of virtual computed columns in the declarative schema changer (DSC). Previously, such operations were not allowed.

A key characteristic of virtual columns is that type changes never require a backfill or column rewrite, as their values are computed dynamically during access. This change includes adjustments to handle type changes that would typically involve a rewrite as simple metadata updates instead.

Additionally, the USING expression in ALTER TABLE statements is now explicitly disallowed for virtual columns, as no data rewriting occurs. Attempts to use this option will result in an error.

Epic: CRDB-25314
Closes: cockroachdb#125840
Release note: none

Co-authored-by: Matt Spilchen <[email protected]>
  • Loading branch information
@spilchen
craig[bot] and spilchen committed Nov 20, 2024
2 parents 354444d + c19872f commit 70d3de4
Show file tree
Hide file tree
Showing 8 changed files with 337 additions and 46 deletions.
4 changes: 2 additions & 2 deletions pkg/sql/alter_column_type.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,12 @@ func AlterColumnType(
}

typ, err = schemachange.ValidateAlterColumnTypeChecks(ctx, t,
params.EvalContext().Settings, typ, col.IsGeneratedAsIdentity())
params.EvalContext().Settings, typ, col.IsGeneratedAsIdentity(), col.IsVirtual())
if err != nil {
return err
}

kind, err := schemachange.ClassifyConversionFromTree(ctx, t, col.GetType(), typ)
kind, err := schemachange.ClassifyConversionFromTree(ctx, t, col.GetType(), typ, col.IsVirtual())
if err != nil {
return err
}
Expand Down
211 changes: 201 additions & 10 deletions pkg/sql/logictest/testdata/logic_test/alter_column_type
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1389,7 +1389,7 @@ SELECT * FROM stored1 ORDER BY A;
statement ok
INSERT INTO stored1 VALUES (2147483648),(2147483647);

skipif config local-legacy-schema-changer local-mixed-24.1 local-mixed-24.2
skipif config local-legacy-schema-changer local-mixed-24.2
statement error pq: validate check constraint: integer out of range for type int4
ALTER TABLE stored1 ALTER COLUMN COMP1 SET DATA TYPE INT4;

Expand All @@ -1401,11 +1401,11 @@ ALTER TABLE stored1 ALTER COLUMN COMP1 SET DATA TYPE INT4;
statement ok
DELETE FROM stored1 WHERE a = 2147483648;

skipif config local-legacy-schema-changer local-mixed-24.1 local-mixed-24.2
skipif config local-legacy-schema-changer local-mixed-24.2
statement ok
ALTER TABLE stored1 ALTER COLUMN COMP1 SET DATA TYPE INT4;

skipif config local-legacy-schema-changer local-mixed-24.1 local-mixed-24.2
skipif config local-legacy-schema-changer local-mixed-24.2
query TT
SHOW CREATE TABLE stored1;
----
Expand All @@ -1416,7 +1416,7 @@ stored1 CREATE TABLE public.stored1 (
FAMILY f1 (a, comp1)
)

skipif config local-legacy-schema-changer local-mixed-24.1
skipif config local-legacy-schema-changer
query II
SELECT * FROM stored1 ORDER BY A;
----
Expand All @@ -1426,25 +1426,25 @@ SELECT * FROM stored1 ORDER BY A;
2000 2000
2147483647 2147483647

skipif config local-legacy-schema-changer local-mixed-24.1 local-mixed-24.2
skipif config local-legacy-schema-changer local-mixed-24.2
# Attempt to convert to a type that is incompatible with the computed expression
statement error pq: expected STORED COMPUTED COLUMN expression to have type bool, but 'a' has type int
ALTER TABLE stored1 ALTER COLUMN comp1 SET DATA TYPE BOOL;

skipif config local-legacy-schema-changer local-mixed-24.1 local-mixed-24.2
skipif config local-legacy-schema-changer local-mixed-24.2
statement error pq: expected STORED COMPUTED COLUMN expression to have type string, but 'a' has type int
ALTER TABLE stored1 ALTER COLUMN comp1 SET DATA TYPE TEXT;

# Convert the type to something compatible, but specify a custom value for the
# column with the USING expression. This will force a type conversion with a backfill.
skipif config local-legacy-schema-changer local-mixed-24.1 local-mixed-24.2 local-mixed-24.3
skipif config local-legacy-schema-changer local-mixed-24.2 local-mixed-24.3
statement ok
ALTER TABLE stored1 ALTER COLUMN comp1 SET DATA TYPE INT2 USING -1;

statement ok
INSERT INTO stored1 VALUES (-1000);

skipif config local-legacy-schema-changer local-mixed-24.1 local-mixed-24.2 local-mixed-24.3
skipif config local-legacy-schema-changer local-mixed-24.2 local-mixed-24.3
query TT
SHOW CREATE TABLE stored1;
----
Expand All @@ -1455,7 +1455,7 @@ stored1 CREATE TABLE public.stored1 (
FAMILY f1 (a, comp1)
)

skipif config local-legacy-schema-changer local-mixed-24.1 local-mixed-24.2 local-mixed-24.3
skipif config local-legacy-schema-changer local-mixed-24.2 local-mixed-24.3
query II
SELECT * FROM stored1 ORDER BY A;
----
Expand All @@ -1467,11 +1467,202 @@ SELECT * FROM stored1 ORDER BY A;
2147483647 -1

# Attempt to drop stored along with changing the column type
skipif config local-legacy-schema-changer local-mixed-24.1 local-mixed-24.2 local-mixed-24.3
skipif config local-legacy-schema-changer local-mixed-24.2 local-mixed-24.3
statement error pq: unimplemented: ALTER COLUMN TYPE cannot be used in combination with other ALTER TABLE commands
ALTER TABLE stored1 ALTER COLUMN comp1 SET DATA TYPE INT4 USING -1, ALTER COLUMN comp1 drop stored;

statement ok
DROP TABLE stored1;

subtest virtual_compute

statement ok
CREATE TABLE virt1 (c1 BIGINT NOT NULL PRIMARY KEY, v1 BIGINT NOT NULL AS (c1) virtual);

statement ok
INSERT INTO virt1 VALUES (100), (2147483647);

# Ensure the USING expression cannot be used
statement error pq: type change for virtual column "v1" cannot be altered with a USING expression
ALTER TABLE virt1 ALTER COLUMN v1 SET DATA TYPE INT4 USING 10;

skipif config local-legacy-schema-changer local-mixed-24.2
statement ok
ALTER TABLE virt1 ALTER COLUMN v1 SET DATA TYPE INT4;

skipif config local-legacy-schema-changer local-mixed-24.2
query II
SELECT * from virt1 ORDER BY c1;
----
100 100
2147483647 2147483647

skipif config local-legacy-schema-changer local-mixed-24.2
query TT
SHOW CREATE TABLE virt1;
----
virt1 CREATE TABLE public.virt1 (
c1 INT8 NOT NULL,
v1 INT4 NOT NULL AS (c1) VIRTUAL,
CONSTRAINT virt1_pkey PRIMARY KEY (c1 ASC)
)

skipif config local-legacy-schema-changer local-mixed-24.2
statement error pq: validate check constraint: integer out of range for type int2
ALTER TABLE virt1 ALTER COLUMN v1 SET DATA TYPE INT2;

statement ok
DELETE FROM virt1 WHERE c1 = 2147483647;

skipif config local-legacy-schema-changer local-mixed-24.2
statement ok
ALTER TABLE virt1 ALTER COLUMN v1 SET DATA TYPE INT2;

statement ok
INSERT INTO virt1 VALUES (-9999);

skipif config local-legacy-schema-changer local-mixed-24.2
query II
SELECT * from virt1 ORDER BY c1;
----
-9999 -9999
100 100

skipif config local-legacy-schema-changer local-mixed-24.2
query TT
SHOW CREATE TABLE virt1;
----
virt1 CREATE TABLE public.virt1 (
c1 INT8 NOT NULL,
v1 INT2 NOT NULL AS (c1) VIRTUAL,
CONSTRAINT virt1_pkey PRIMARY KEY (c1 ASC)
)

skipif config local-legacy-schema-changer local-mixed-24.2
statement error pq: expected STORED COMPUTED COLUMN expression to have type string, but 'c1' has type int
ALTER TABLE virt1 ALTER COLUMN v1 SET DATA TYPE TEXT;

skipif config local-legacy-schema-changer local-mixed-24.2
statement error pq: expected STORED COMPUTED COLUMN expression to have type float, but 'c1' has type int
ALTER TABLE virt1 ALTER COLUMN v1 SET DATA TYPE FLOAT;

statement ok
DROP TABLE virt1;

subtest virtual_compute_tz

statement ok
CREATE TABLE virt2 (C1 TIMESTAMP(6) NOT NULL PRIMARY KEY, v1 TIMESTAMP(3) AS (c1) VIRTUAL);

statement ok
INSERT INTO virt2 VALUES ('2024-10-31 16:50:00.123456');

query TT
SELECT * FROM virt2 ORDER BY c1;
----
2024-10-31 16:50:00.123456 +0000 +0000 2024-10-31 16:50:00.123 +0000 +0000

statement ok
ALTER TABLE virt2 ALTER COLUMN v1 SET DATA TYPE TIMESTAMP(2);

query TT
SELECT * FROM virt2 ORDER BY c1;
----
2024-10-31 16:50:00.123456 +0000 +0000 2024-10-31 16:50:00.12 +0000 +0000

query TT
SHOW CREATE TABLE virt2;
----
virt2 CREATE TABLE public.virt2 (
c1 TIMESTAMP(6) NOT NULL,
v1 TIMESTAMP(2) NULL AS (c1) VIRTUAL,
CONSTRAINT virt2_pkey PRIMARY KEY (c1 ASC)
)

statement ok
ALTER TABLE virt2 ALTER COLUMN v1 SET DATA TYPE TIMESTAMP(5);

query TT
SELECT * FROM virt2 ORDER BY c1;
----
2024-10-31 16:50:00.123456 +0000 +0000 2024-10-31 16:50:00.12346 +0000 +0000

query TT
SHOW CREATE TABLE virt2;
----
virt2 CREATE TABLE public.virt2 (
c1 TIMESTAMP(6) NOT NULL,
v1 TIMESTAMP(5) NULL AS (c1) VIRTUAL,
CONSTRAINT virt2_pkey PRIMARY KEY (c1 ASC)
)

statement ok
DROP TABLE virt2;

subtest virtual_compute_decimal

statement ok
CREATE TABLE virt3 (C1 DECIMAL(6,3) NOT NULL PRIMARY KEY, v1 DECIMAL(6,3) AS (c1) VIRTUAL);

statement ok
INSERT INTO virt3 VALUES (1.23456),(12.34567),(-123.4);

query TT
SELECT * FROM virt3 ORDER BY c1;
----
-123.400 -123.400
1.235 1.235
12.346 12.346

skipif config local-legacy-schema-changer local-mixed-24.2
statement ok
ALTER TABLE virt3 ALTER COLUMN v1 SET DATA TYPE DECIMAL(6,2);

skipif config local-legacy-schema-changer local-mixed-24.2
query TT
SELECT * FROM virt3 ORDER BY c1;
----
-123.400 -123.40
1.235 1.24
12.346 12.35

skipif config local-legacy-schema-changer local-mixed-24.2
query TT
SHOW CREATE TABLE virt3;
----
virt3 CREATE TABLE public.virt3 (
c1 DECIMAL(6,3) NOT NULL,
v1 DECIMAL(6,2) NULL AS (c1) VIRTUAL,
CONSTRAINT virt3_pkey PRIMARY KEY (c1 ASC)
)

skipif config local-legacy-schema-changer local-mixed-24.2
statement error pq: validate check constraint: .*value with precision 5, scale 4 must round to an absolute value less than 10\^1
ALTER TABLE virt3 ALTER COLUMN v1 SET DATA TYPE DECIMAL(5,4);

skipif config local-legacy-schema-changer local-mixed-24.2
statement ok
ALTER TABLE virt3 ALTER COLUMN v1 SET DATA TYPE DECIMAL(8,4);

skipif config local-legacy-schema-changer local-mixed-24.2
query TT
SELECT * FROM virt3 ORDER BY c1;
----
-123.400 -123.4000
1.235 1.2350
12.346 12.3460

skipif config local-legacy-schema-changer local-mixed-24.2
query TT
SHOW CREATE TABLE virt3;
----
virt3 CREATE TABLE public.virt3 (
c1 DECIMAL(6,3) NOT NULL,
v1 DECIMAL(8,4) NULL AS (c1) VIRTUAL,
CONSTRAINT virt3_pkey PRIMARY KEY (c1 ASC)
)

statement ok
DROP TABLE virt3;

subtest end
1 change: 1 addition & 0 deletions pkg/sql/schemachange/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ go_test(
"//pkg/util/log",
"//pkg/util/randutil",
"//pkg/util/uuid",
"@com_github_stretchr_testify//require",
],
)

Expand Down
60 changes: 58 additions & 2 deletions pkg/sql/schemachange/alter_column_type.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,30 @@ var classifiers = map[types.Family]map[types.Family]classifier{
},
}

// virtualGeneralReclassifier is used to classify general conversions for virtual
// computed columns. General conversions don’t apply to these columns, as they
// aren’t physically stored on disk. If this map is used and the type family is
// missing, it’s assumed that the type conversion cannot be applied.
var virtualGeneralReclassifier = map[types.Family]map[types.Family]classifier{
types.DecimalFamily: {
types.DecimalFamily: ColumnConversionTrivial.classifier(),
},
types.TimestampFamily: {
types.TimestampTZFamily: ColumnConversionTrivial.classifier(),
types.TimestampFamily: ColumnConversionTrivial.classifier(),
},
types.TimestampTZFamily: {
types.TimestampFamily: ColumnConversionTrivial.classifier(),
types.TimestampTZFamily: ColumnConversionTrivial.classifier(),
},
types.TimeFamily: {
types.TimeFamily: ColumnConversionTrivial.classifier(),
},
types.TimeTZFamily: {
types.TimeTZFamily: ColumnConversionTrivial.classifier(),
},
}

// classifierHardestOf creates a composite classifier that returns the
// hardest kind of the enclosed classifiers. If any of the
// classifiers report impossible, impossible will be returned.
Expand Down Expand Up @@ -246,15 +270,39 @@ func ClassifyConversion(
// ClassifyConversionFromTree is a wrapper for ClassifyConversion when we want
// to take into account the parsed AST for ALTER TABLE .. ALTER COLUMN.
func ClassifyConversionFromTree(
ctx context.Context, t *tree.AlterTableAlterColumnType, oldType *types.T, newType *types.T,
ctx context.Context,
t *tree.AlterTableAlterColumnType,
oldType *types.T,
newType *types.T,
isVirtual bool,
) (ColumnConversionKind, error) {
if t.Using != nil {
// If an expression is provided, we always need to try a general conversion.
// We have to follow the process to create a new column and backfill it
// using the expression.
return ColumnConversionGeneral, nil
}
return ClassifyConversion(ctx, oldType, newType)
kind, err := ClassifyConversion(ctx, oldType, newType)
if err != nil {
return kind, err
}
// A general rewrite isn't applicable for virtual columns since they don’t exist
// physically. We need to pick a new classifier. For conversions that would require
// general handling due to incompatible type families (e.g., INT -> TEXT), we
// assume these will already be rejected because the computed expression doesn’t
// match the new type. Such cases are handled by validateNewTypeForComputedColumn.
if isVirtual && kind == ColumnConversionGeneral {
if inner, oldTypeFamilyFound := virtualGeneralReclassifier[oldType.Family()]; oldTypeFamilyFound {
if fn, newTypeFamilyFound := inner[newType.Family()]; newTypeFamilyFound {
kind = fn(oldType, newType)
return kind, nil
}
}
return ColumnConversionImpossible,
pgerror.Newf(pgcode.CannotCoerce, "cannot convert %s to %s for a virtual column",
oldType.SQLString(), newType.SQLString())
}
return kind, nil
}

// ValidateAlterColumnTypeChecks performs validation checks on the proposed type
Expand All @@ -267,6 +315,7 @@ func ValidateAlterColumnTypeChecks(
settions *cluster.Settings,
origTyp *types.T,
isGeneratedAsIdentity bool,
isVirtual bool,
) (*types.T, error) {
typ := origTyp
// Special handling for STRING COLLATE xy to verify that we recognize the language.
Expand All @@ -286,5 +335,12 @@ func ValidateAlterColumnTypeChecks(
}
}

// A USING expression is unnecessary when altering the type of a virtual column,
// as its value is always computed at runtime and is not stored on disk.
if isVirtual && t.Using != nil {
return typ, pgerror.Newf(pgcode.FeatureNotSupported,
"type change for virtual column %q cannot be altered with a USING expression", t.Column)
}

return typ, colinfo.ValidateColumnDefType(ctx, settions, typ)
}
Loading

0 comments on commit 70d3de4

Please sign in to comment.