-
Notifications
You must be signed in to change notification settings - Fork 711
Redshift alter column type no set #1912
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
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1060,6 +1060,22 @@ pub trait Dialect: Debug + Any { | |||||||||||||
| fn supports_space_separated_column_options(&self) -> bool { | ||||||||||||||
| false | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /// Returns true if the dialect supports `ALTER TABLE tbl ALTER COLUMN col TYPE <type>` | ||||||||||||||
| /// without specifying `SET DATA` before `TYPE`. | ||||||||||||||
| /// | ||||||||||||||
| /// - [Redshift](https://docs.aws.amazon.com/redshift/latest/dg/r_ALTER_TABLE.html#r_ALTER_TABLE-synopsis) | ||||||||||||||
| /// - [PostgreSQL](https://www.postgresql.org/docs/current/sql-altertable.html) | ||||||||||||||
| fn supports_alter_column_type_without_set(&self) -> bool { | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure I followed that this flag is necessary, it looks like we would be able to accept either variant with or without the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes that is an option. I hesitated a bit because saw specific negative tests, and did not want to break anything. |
||||||||||||||
| false | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /// Returns true if the dialect supports `ALTER TABLE tbl ALTER COLUMN col SET DATA TYPE <type> USING <exp>` | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
thinking something like this so that its a bit clearer which part of the statement the flag refers to? |
||||||||||||||
| /// | ||||||||||||||
| /// - [PostgreSQL](https://www.postgresql.org/docs/current/sql-altertable.html) | ||||||||||||||
| fn supports_alter_column_type_using(&self) -> bool { | ||||||||||||||
| false | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /// This represents the operators for which precedence must be defined | ||||||||||||||
|
|
||||||||||||||
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.