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.
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
sql: only allow altering a column type when an assignment cast is allowed #115442
Conversation
Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR. My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
Thank you for updating your pull request. My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
c18ddff
to
6495725
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for your contribution!
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @andrew-delph and @mgartner)
-- commits
line 9 at r1:
nit: "postures" -> "postgres"
pkg/sql/alter_column_type.go
line 342 at r1 (raw file):
// Validate that the column can be automatically cast to toType without explicit casting. if validCast := cast.ValidCast(col.GetType(), toType, cast.ContextAssignment); !validCast { return pgerror.WithCandidateCode(
nit: use this constructor:
return errors.WithHintf(
pgerror.Newf(
pgcode.DatatypeMismatch,
"column %q cannot be cast automatically to type %s",
col.GetName(),
toType.SQLString(),
), "You might need to specify \"USING %s\".", s,
)
Looks like there are failing UTs, so I will take a look. |
d68a009
to
2450ec6
Compare
…owed Fixes: cockroachdb#82416 Release note (sql change): Reject changing column types unless an explicitly cast is provided where cannot automatically cast. This replicates logic in postgres. Prior to this change, casts such as BOOL to INT were possible without explicitly casting.
Friendly ping @mgartner |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 2 files at r1, 1 of 1 files at r2, 4 of 4 files at r3, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @andrew-delph and @rafiss)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
bors r+
Build succeeded! And happy new year! 🎉 |
Fixes: #82416
Release note (sql change): Reject changing column types unless an explicitly cast is provided where cannot automatically cast. This replicates logic in Postgres. Prior to this change, casts such as BOOL to INT were possible without explicitly casting.