iffyio commented on code in PR #1467:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1467#discussion_r1798046143
##########
src/parser/mod.rs:
##########
@@ -11173,15 +11173,42 @@ impl<'a> Parser<'a> {
self.peek_token().location
)
}
- expr => self
- .parse_optional_alias(keywords::RESERVED_FOR_COLUMN_ALIAS)
- .map(|alias| match alias {
- Some(alias) => SelectItem::ExprWithAlias { expr, alias },
- None => SelectItem::UnnamedExpr(expr),
- }),
+ expr => {
+ if dialect_of!(self is MsSqlDialect) {
Review Comment:
We recently started to discourage the use of
[dialect_of](https://github.com/apache/datafusion-sqlparser-rs/blob/383226c3e81d9e5ed64f6f188815dfc9d7fea317/src/dialect/mod.rs#L63-L72)
can we update this to use a method on the Dialect trait as described in the
link?
We could add a method like `supports_eq_alias_assigment` or similar
##########
src/parser/mod.rs:
##########
@@ -11173,15 +11173,42 @@ impl<'a> Parser<'a> {
self.peek_token().location
)
}
- expr => self
- .parse_optional_alias(keywords::RESERVED_FOR_COLUMN_ALIAS)
- .map(|alias| match alias {
- Some(alias) => SelectItem::ExprWithAlias { expr, alias },
- None => SelectItem::UnnamedExpr(expr),
- }),
+ expr => {
+ if dialect_of!(self is MsSqlDialect) {
+ if let Some(select_item) =
self.parse_mssql_alias_with_equal(&expr) {
+ return Ok(select_item);
+ }
+ }
+ self.parse_optional_alias(keywords::RESERVED_FOR_COLUMN_ALIAS)
+ .map(|alias| match alias {
+ Some(alias) => SelectItem::ExprWithAlias { expr, alias
},
+ None => SelectItem::UnnamedExpr(expr),
+ })
+ }
}
}
+ /// Parse a [`SelectItem`] based on an MsSql syntax that uses the equal
sign
+ /// to denote an alias, for example: SELECT col_alias = col FROM tbl
+ ///
<https://learn.microsoft.com/en-us/sql/t-sql/queries/select-examples-transact-sql?view=sql-server-ver16#b-use-select-with-column-headings-and-calculations>
Review Comment:
```suggestion
/// Parse a [`SelectItem`] based on an [MsSql] syntax that uses the
equal sign
/// to denote an alias, for example: SELECT col_alias = col FROM tbl
/// [MsSql]:
https://learn.microsoft.com/en-us/sql/t-sql/queries/select-examples-transact-sql?view=sql-server-ver16#b-use-select-with-column-headings-and-calculations
```
##########
src/parser/mod.rs:
##########
@@ -11173,15 +11173,42 @@ impl<'a> Parser<'a> {
self.peek_token().location
)
}
- expr => self
- .parse_optional_alias(keywords::RESERVED_FOR_COLUMN_ALIAS)
- .map(|alias| match alias {
- Some(alias) => SelectItem::ExprWithAlias { expr, alias },
- None => SelectItem::UnnamedExpr(expr),
- }),
+ expr => {
+ if dialect_of!(self is MsSqlDialect) {
+ if let Some(select_item) =
self.parse_mssql_alias_with_equal(&expr) {
+ return Ok(select_item);
+ }
+ }
+ self.parse_optional_alias(keywords::RESERVED_FOR_COLUMN_ALIAS)
+ .map(|alias| match alias {
+ Some(alias) => SelectItem::ExprWithAlias { expr, alias
},
+ None => SelectItem::UnnamedExpr(expr),
+ })
+ }
}
}
+ /// Parse a [`SelectItem`] based on an MsSql syntax that uses the equal
sign
+ /// to denote an alias, for example: SELECT col_alias = col FROM tbl
+ ///
<https://learn.microsoft.com/en-us/sql/t-sql/queries/select-examples-transact-sql?view=sql-server-ver16#b-use-select-with-column-headings-and-calculations>
+ fn parse_mssql_alias_with_equal(&mut self, expr: &Expr) ->
Option<SelectItem> {
+ if let Expr::BinaryOp {
+ left, op, right, ..
+ } = expr
+ {
+ if op == &BinaryOperator::Eq {
+ if let Expr::Identifier(ref alias) = **left {
+ return Some(SelectItem::ExprWithAlias {
+ expr: *right.clone(),
+ alias: alias.clone(),
+ });
+ }
+ }
+ }
+
+ None
Review Comment:
```suggestion
if let Expr::BinaryOp {
left: Expr::Identifier(alias), op: BinaryOperator::Eq, right,
} = expr {
SelectItem::ExprWithAlias {
expr: right,
alias: Expr::Identifier(alias),
}
} else {
expr
}
```
Could be simplified to the following? I think no need to pass in a reference
and return an option, we can move the value and return the same value if
untouched?
Also we can skip the `..` operator when deconstructing in this case, so that
if the field set changes, we would be notified to ensure the logic remains
correct.
##########
src/parser/mod.rs:
##########
@@ -11173,15 +11173,42 @@ impl<'a> Parser<'a> {
self.peek_token().location
)
}
- expr => self
- .parse_optional_alias(keywords::RESERVED_FOR_COLUMN_ALIAS)
- .map(|alias| match alias {
- Some(alias) => SelectItem::ExprWithAlias { expr, alias },
- None => SelectItem::UnnamedExpr(expr),
- }),
+ expr => {
+ if dialect_of!(self is MsSqlDialect) {
+ if let Some(select_item) =
self.parse_mssql_alias_with_equal(&expr) {
+ return Ok(select_item);
+ }
+ }
+ self.parse_optional_alias(keywords::RESERVED_FOR_COLUMN_ALIAS)
+ .map(|alias| match alias {
+ Some(alias) => SelectItem::ExprWithAlias { expr, alias
},
+ None => SelectItem::UnnamedExpr(expr),
+ })
+ }
}
}
+ /// Parse a [`SelectItem`] based on an MsSql syntax that uses the equal
sign
+ /// to denote an alias, for example: SELECT col_alias = col FROM tbl
+ ///
<https://learn.microsoft.com/en-us/sql/t-sql/queries/select-examples-transact-sql?view=sql-server-ver16#b-use-select-with-column-headings-and-calculations>
+ fn parse_mssql_alias_with_equal(&mut self, expr: &Expr) ->
Option<SelectItem> {
Review Comment:
```suggestion
fn maybe_unpack_alias_assignment(expr: &Expr) -> Option<SelectItem> {
```
Thinking since this is only a helper function that it doesn't get confused
for a parser/parsing method
--
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]
