iffyio commented on code in PR #1914:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1914#discussion_r2177383388
##########
src/ast/mod.rs:
##########
@@ -3316,6 +3316,18 @@ pub enum Statement {
options: Vec<SecretOption>,
},
/// ```sql
+ /// CREATE SERVER
+ /// ```
+ /// See
[PostgreSQL](https://www.postgresql.org/docs/current/sql-createserver.html)
+ CreateServer {
+ name: ObjectName,
+ if_not_exists: bool,
+ server_type: Option<Ident>,
+ version: Option<Ident>,
+ fdw_name: ObjectName,
Review Comment:
Can we rename this to something more descriptive e.g. explicitly
`foreign_data_wrapper`?
##########
src/parser/mod.rs:
##########
@@ -15806,6 +15810,53 @@ impl<'a> Parser<'a> {
Ok(sequence_options)
}
+ /// ```sql
+ /// CREATE SERVER [ IF NOT EXISTS ] server_name [ TYPE 'server_type' ]
[ VERSION 'server_version' ]
+ /// FOREIGN DATA WRAPPER fdw_name
+ /// [ OPTIONS ( option 'value' [, ... ] ) ]
+ /// ```
+ ///
+ /// [PostgreSQL
Documentation](https://www.postgresql.org/docs/current/sql-createserver.html)
+ pub fn parse_pg_create_server(&mut self) -> Result<Statement, ParserError>
{
Review Comment:
```suggestion
/// Parse a `CREATE SERVER` statement.
///
/// See [Statement::CreateServer]
pub fn parse_pg_create_server(&mut self) -> Result<Statement,
ParserError> {
```
I think we can do something like this to avoid duplicating the comments. The
syntax we can skip entirely since that should be covered by the doc link if the
reader requires further context
##########
src/ast/mod.rs:
##########
@@ -7973,6 +8015,20 @@ impl fmt::Display for SecretOption {
}
}
+#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
+pub struct ServerOption {
Review Comment:
```suggestion
pub struct CreateServerOption {
```
##########
src/ast/mod.rs:
##########
@@ -3316,6 +3316,18 @@ pub enum Statement {
options: Vec<SecretOption>,
},
/// ```sql
+ /// CREATE SERVER
+ /// ```
+ /// See
[PostgreSQL](https://www.postgresql.org/docs/current/sql-createserver.html)
+ CreateServer {
Review Comment:
For new statements [we're moving away from using anonymous
structs](https://github.com/apache/datafusion-sqlparser-rs/issues/1204), here
can we introduce a dedicated struct for this statement?
```rust
CreateServer(CreateServerStatement)
```
See
[Statement::Raise](https://github.com/apache/datafusion-sqlparser-rs/blob/9d68a5663dcdb0357ee52bc351dd99a5905fbec5/src/ast/mod.rs#L3145-L3146)
as an example
##########
src/parser/mod.rs:
##########
@@ -4662,6 +4662,10 @@ impl<'a> Parser<'a> {
self.parse_create_procedure(or_alter)
} else if self.parse_keyword(Keyword::CONNECTOR) {
self.parse_create_connector()
+ } else if self.parse_keyword(Keyword::SERVER)
+ && dialect_of!(self is PostgreSqlDialect | GenericDialect)
Review Comment:
```suggestion
```
I think it should be fine to let the parser be permissive and accept this
statement whenever it shows up
--
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]
