iffyio commented on code in PR #1490:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1490#discussion_r1825432921
##########
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:
That sounds reasonable to me
##########
tests/sqlparser_mssql.rs:
##########
@@ -570,6 +570,41 @@ fn parse_substring_in_select() {
}
}
+#[test]
+fn parse_mssql_execute_stored_procedure() {
Review Comment:
Not gating the behavior sounds reasonable! I think we should move the tests
to common in that case so that the tests cover the parser behavior. otherwise
could happen that some variant is being correctly parsed for some dialect being
relied on but since we have no test coverage for that dialect, we could end up
breaking it in the future if guards are added or the code changes. Also
currently its a bit hidden like with the `has_parenthesis` thread not knowing
which dialects are valid for which variants of the statement, even though the
parser accepts both.
##########
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:
Ah I think that can probably be resolved more conventionally using
[ms_and_generic().one_statement_parses_to(sql,
canonical)](https://github.com/apache/datafusion-sqlparser-rs/blob/main/src/test_utils.rs#L154)?
##########
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:
Ah alright I thought for some reason the parenthesis was optional for mysql
introduced in the MR
--
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]
