iffyio commented on code in PR #1490:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1490#discussion_r1824245446
##########
tests/sqlparser_mssql.rs:
##########
@@ -570,6 +570,41 @@ fn parse_substring_in_select() {
}
}
+#[test]
+fn parse_mssql_execute_stored_procedure() {
+ let expected = Statement::Execute {
+ name: ObjectName(vec![
+ Ident {
+ value: "my_schema".to_string(),
+ quote_style: None,
+ },
+ Ident {
+ value: "my_stored_procedure".to_string(),
+ quote_style: None,
+ },
+ ]),
+ parameters: vec![
+ Expr::Value(Value::NationalStringLiteral("param1".to_string())),
+ Expr::Value(Value::NationalStringLiteral("param2".to_string())),
+ ],
+ has_parentheses: false,
Review Comment:
Can we add a test case that exercises the `has_parenthesis` codepath
(looking at the test sqls none seem to use parenthesis)?
##########
tests/sqlparser_mssql.rs:
##########
@@ -570,6 +570,41 @@ fn parse_substring_in_select() {
}
}
+#[test]
+fn parse_mssql_execute_stored_procedure() {
+ let expected = Statement::Execute {
+ name: ObjectName(vec![
+ Ident {
+ value: "my_schema".to_string(),
+ quote_style: None,
+ },
+ Ident {
+ value: "my_stored_procedure".to_string(),
+ quote_style: None,
+ },
+ ]),
+ parameters: vec![
+ Expr::Value(Value::NationalStringLiteral("param1".to_string())),
+ Expr::Value(Value::NationalStringLiteral("param2".to_string())),
+ ],
+ has_parentheses: false,
+ using: vec![],
+ };
+ assert_eq!(
+ ms().verified_stmt("EXECUTE my_schema.my_stored_procedure N'param1',
N'param2'"),
+ expected
+ );
+ assert_eq!(
+ Parser::parse_sql(
+ &MsSqlDialect {},
+ "EXEC my_schema.my_stored_procedure N'param1', N'param2';"
+ )
Review Comment:
was it something specific that required using this pattern vs
`ms().verified*`?
##########
tests/sqlparser_mssql.rs:
##########
@@ -570,6 +570,41 @@ fn parse_substring_in_select() {
}
}
+#[test]
+fn parse_mssql_execute_stored_procedure() {
Review Comment:
If the code isn't restricted to the mssql dialect can we move this to common
so that we cover all dialects that we parse for (or if its mssql specific we
can guard the behavior)? Thinking so that we avoid regressions where features
unintentionally get removed from dialects due to lacking coverage on that
dialect
##########
src/ast/mod.rs:
##########
@@ -3095,10 +3095,14 @@ pub enum Statement {
/// EXECUTE name [ ( parameter [, ...] ) ] [USING <expr>]
/// ```
///
- /// Note: this is a PostgreSQL-specific statement.
+ /// Note: this statement is supported by Postgres and MSSQL, with slight
differences in syntax.
+ ///
+ /// Postgres: <https://www.postgresql.org/docs/current/sql-execute.html>
+ /// MSSQL:
<https://learn.microsoft.com/en-us/sql/relational-databases/stored-procedures/execute-a-stored-procedure>
Execute {
- name: Ident,
+ name: ObjectName,
parameters: Vec<Expr>,
+ has_parentheses: bool,
Review Comment:
would it be reasonable to use a wrapper similar to `OneOrManyWithParens` to
represent this pattern? e.g. `MaybeParens<T>` - feels like this wouldn't be a
one-off and the flag feels not ideal since it doesn't contain data and its not
super clear what parenthesis refers to without reading the code
--
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]
