Skip to content

Commit

Permalink
sql/schemachanger: Support enum types for DSC impl of ALTER COLUMN ..…
Browse files Browse the repository at this point in the history
… ALTER TYPE

Previously, attempts to alter a column's type to an enum were blocked due to
failures during the backfill process, resulting in the error: "cannot resolve
types in DistSQL by name." This change addresses that issue by ensuring all
type names are fully resolved when populating the USING expression for the
alteration.

Additionally, I've corrected an error message for column type conversions
involving an enum type without a USING clause. The previous message included a
hint with an unhydrated type name, which was confusing for users. The hint used
to look like this:
```
HINT: You might need to specify "USING: x::@100117".
```

This fix now provides a more meaningful hint by displaying the correct type
name.

Epic: CRDB-25314
Closes cockroachdb#132936
Release note: none
  • Loading branch information
spilchen committed Oct 29, 2024
1 parent ba425ba commit b2e21b2
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 19 deletions.
19 changes: 16 additions & 3 deletions pkg/sql/logictest/testdata/logic_test/enums
Original file line number Diff line number Diff line change
Expand Up @@ -1009,17 +1009,30 @@ statement ok
SET enable_experimental_alter_column_type_general = true;

statement ok
CREATE TABLE enum_data_type (x STRING);
CREATE TABLE enum_data_type (x STRING, y INT);
INSERT INTO enum_data_type VALUES ('hello'), ('howdy');

skipif config local-legacy-schema-changer "legacy doesn't hydrate the type name in the message"
skipif config local-mixed-24.1 "same as above. 24.1 doesn't have any ALTER TYPE support in the DSC, so it's legacy"
statement error pq: column "y" cannot be cast automatically to type public.greeting\nHINT: You might need to specify "USING y::public.greeting".
ALTER TABLE enum_data_type ALTER COLUMN y SET DATA TYPE greeting;

statement error pq: invalid cast: int -> greeting
ALTER TABLE enum_data_type ALTER COLUMN y SET DATA TYPE greeting USING y::greeting;

skipif config local-legacy-schema-changer "legacy doesn't hydrate the type name in the message"
skipif config local-mixed-24.1 "same as above. 24.1 doesn't have any ALTER TYPE support in the DSC, so it's legacy"
statement error pq: column "x" cannot be cast automatically to type public.greeting\nHINT: You might need to specify "USING x::public.greeting".
ALTER TABLE enum_data_type ALTER COLUMN x SET DATA TYPE greeting;

statement ok
ALTER TABLE enum_data_type ALTER COLUMN x SET DATA TYPE greeting USING x::greeting;

statement ok
INSERT INTO enum_data_type VALUES ('hi')

query T rowsort
SELECT * FROM enum_data_type
SELECT x FROM enum_data_type
----
hello
howdy
Expand Down Expand Up @@ -1090,7 +1103,7 @@ statement ok
CREATE TABLE enum_data_type (x STRING);
INSERT INTO enum_data_type VALUES ('notagreeting')

statement error pq: invalid input value for enum greeting: "notagreeting"
statement error pq: .*invalid input value for enum greeting: "notagreeting"
ALTER TABLE enum_data_type ALTER COLUMN x SET DATA TYPE greeting USING x::greeting

statement ok
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ func alterTableAlterColumnType(
panic(err)
}

validateAutomaticCastForNewType(b, tbl.TableID, colID, t.Column.String(),
oldColType.Type, newColType.Type, t.Using != nil)
validateAutomaticCastForNewType(b, tbl.TableID, oldColType.ColumnID, t.Column.String(),
oldColType.Type, newColType.Type, t.Using != nil, tn)

kind, err := schemachange.ClassifyConversionFromTree(b, t, oldColType.Type, newColType.Type)
if err != nil {
Expand Down Expand Up @@ -117,6 +117,7 @@ func validateAutomaticCastForNewType(
colName string,
fromType, toType *types.T,
hasUsingExpr bool,
tn *tree.TableName,
) {
if validCast := cast.ValidCast(fromType, toType, cast.ContextAssignment); validCast {
return
Expand All @@ -126,18 +127,17 @@ func validateAutomaticCastForNewType(
// suggested hint to use one.
if !hasUsingExpr {
// Compute a suggested default computed expression for inclusion in the error hint.
hintExpr := tree.CastExpr{
Expr: &tree.ColumnItem{ColumnName: tree.Name(colName)},
Type: toType,
SyntaxMode: tree.CastShort,
hintExpr, err := parser.ParseExpr(fmt.Sprintf("%s::%s", colName, toType.SQLString()))
if err != nil {
panic(err)
}
panic(errors.WithHintf(
pgerror.Newf(
pgcode.DatatypeMismatch,
"column %q cannot be cast automatically to type %s",
colName,
toType.SQLString(),
), "You might need to specify \"USING %s\".", tree.Serialize(&hintExpr),
), "You might need to specify \"USING %s\".", tree.Serialize(hintExpr),
))
}

Expand Down Expand Up @@ -284,14 +284,6 @@ func handleGeneralColumnConversion(
"old active version; ALTER COLUMN TYPE requires backfill. Reverting to legacy handling"))
}

// TODO(#132936): Not yet supported in the DSC. Throwing an error to trigger
// fallback to legacy.
if newColType.Type.Family() == types.EnumFamily {
panic(scerrors.NotImplementedErrorf(t,
"backfilling during ALTER COLUMN TYPE for an enum column "+
"type is not supported"))
}

// Generate the ID of the new column we are adding.
newColID := b.NextTableColumnID(tbl)
newColType.ColumnID = newColID
Expand Down Expand Up @@ -447,7 +439,7 @@ func getComputeExpressionForBackfill(
return
}

_, _, _, err = schemaexpr.DequalifyAndValidateExprImpl(b, expr, newColType.Type,
typedExpr, _, _, err := schemaexpr.DequalifyAndValidateExprImpl(b, expr, newColType.Type,
tree.AlterColumnTypeUsingExpr, b.SemaCtx(), volatility.Volatile, tn, b.ClusterSettings().Version.ActiveVersion(b),
func() colinfo.ResultColumns {
return getNonDropResultColumns(b, tableID)
Expand All @@ -456,6 +448,14 @@ func getComputeExpressionForBackfill(
return columnLookupFn(b, tableID, columnName)
},
)
if err != nil {
return
}

// Return the updated expression from DequalifyAndValidateExprImpl. For expressions
// involving user-defined types like enums, the types will be resolved to ensure
// they can be used during backfill.
expr, err = parser.ParseExpr(typedExpr)
return
}

Expand Down

0 comments on commit b2e21b2

Please sign in to comment.