iffyio commented on code in PR #1912:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1912#discussion_r2177358792
##########
src/dialect/mod.rs:
##########
@@ -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 {
Review Comment:
not sure I followed that this flag is necessary, it looks like we would be
able to accept either variant with or without the `SET DATA` prefix? The error
message of existing tests if I understood the PR description correctly we can
change to match the new behavior I think
##########
src/dialect/mod.rs:
##########
@@ -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 {
+ false
+ }
+
+ /// Returns true if the dialect supports `ALTER TABLE tbl ALTER COLUMN col
SET DATA TYPE <type> USING <exp>`
Review Comment:
```suggestion
/// Returns true if the dialect supports the `USING` clause in an `ALTER
COLUMN` statement.
/// Example:
/// ```sql
/// ALTER TABLE tbl ALTER COLUMN col SET DATA TYPE <type> USING <exp>`
/// ```
```
thinking something like this so that its a bit clearer which part of the
statement the flag refers to?
##########
src/ast/ddl.rs:
##########
@@ -893,7 +893,10 @@ pub enum AlterColumnOperation {
data_type: DataType,
/// PostgreSQL specific
using: Option<Expr>,
+ /// Whether the statement included the optional `SET DATA` keywords
+ had_set: bool,
Review Comment:
```suggestion
/// Set to true if the statement includes the `SET DATA TYPE`
keywords
set_data_type: bool,
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
