-
Notifications
You must be signed in to change notification settings - Fork 711
Add support for procedure parameter default values #2041
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -16497,7 +16497,8 @@ fn parse_create_procedure_with_parameter_modes() { | |||||||||||||
| span: fake_span, | ||||||||||||||
| }, | ||||||||||||||
| data_type: DataType::Integer(None), | ||||||||||||||
| mode: Some(ArgMode::In) | ||||||||||||||
| mode: Some(ArgMode::In), | ||||||||||||||
| default: None, | ||||||||||||||
| }, | ||||||||||||||
| ProcedureParam { | ||||||||||||||
| name: Ident { | ||||||||||||||
|
|
@@ -16506,7 +16507,8 @@ fn parse_create_procedure_with_parameter_modes() { | |||||||||||||
| span: fake_span, | ||||||||||||||
| }, | ||||||||||||||
| data_type: DataType::Text, | ||||||||||||||
| mode: Some(ArgMode::Out) | ||||||||||||||
| mode: Some(ArgMode::Out), | ||||||||||||||
| default: None, | ||||||||||||||
| }, | ||||||||||||||
| ProcedureParam { | ||||||||||||||
| name: Ident { | ||||||||||||||
|
|
@@ -16515,7 +16517,8 @@ fn parse_create_procedure_with_parameter_modes() { | |||||||||||||
| span: fake_span, | ||||||||||||||
| }, | ||||||||||||||
| data_type: DataType::Timestamp(None, TimezoneInfo::None), | ||||||||||||||
| mode: Some(ArgMode::InOut) | ||||||||||||||
| mode: Some(ArgMode::InOut), | ||||||||||||||
| default: None, | ||||||||||||||
| }, | ||||||||||||||
| ProcedureParam { | ||||||||||||||
| name: Ident { | ||||||||||||||
|
|
@@ -16524,7 +16527,8 @@ fn parse_create_procedure_with_parameter_modes() { | |||||||||||||
| span: fake_span, | ||||||||||||||
| }, | ||||||||||||||
| data_type: DataType::Bool, | ||||||||||||||
| mode: None | ||||||||||||||
| mode: None, | ||||||||||||||
| default: None, | ||||||||||||||
| }, | ||||||||||||||
| ]) | ||||||||||||||
| ); | ||||||||||||||
|
|
@@ -16533,6 +16537,74 @@ fn parse_create_procedure_with_parameter_modes() { | |||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| #[test] | ||||||||||||||
| fn create_procedure_with_parameter_default_value() { | ||||||||||||||
| let sql = r#"CREATE PROCEDURE test_proc (IN a INTEGER = 1, OUT b TEXT = '2', INOUT c TIMESTAMP = NULL, d BOOL = 0) AS BEGIN SELECT 1; END"#; | ||||||||||||||
| match verified_stmt(sql) { | ||||||||||||||
| Statement::CreateProcedure { | ||||||||||||||
| or_alter, | ||||||||||||||
| name, | ||||||||||||||
| params, | ||||||||||||||
| .. | ||||||||||||||
| } => { | ||||||||||||||
| assert_eq!(or_alter, false); | ||||||||||||||
| assert_eq!(name.to_string(), "test_proc"); | ||||||||||||||
| let fake_span = Span { | ||||||||||||||
| start: Location { line: 0, column: 0 }, | ||||||||||||||
| end: Location { line: 0, column: 0 }, | ||||||||||||||
| }; | ||||||||||||||
| assert_eq!( | ||||||||||||||
| params, | ||||||||||||||
| Some(vec![ | ||||||||||||||
| ProcedureParam { | ||||||||||||||
| name: Ident { | ||||||||||||||
| value: "a".into(), | ||||||||||||||
| quote_style: None, | ||||||||||||||
| span: fake_span, | ||||||||||||||
| }, | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I think this can be simplified? that would let us skip introducing the fake span as well
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, certainly. I did it this way because that's how the similar modes test did it, but I will simplify |
||||||||||||||
| data_type: DataType::Integer(None), | ||||||||||||||
| mode: Some(ArgMode::In), | ||||||||||||||
| default: Some(Expr::Value((number("1")).with_empty_span())), | ||||||||||||||
| }, | ||||||||||||||
| ProcedureParam { | ||||||||||||||
| name: Ident { | ||||||||||||||
| value: "b".into(), | ||||||||||||||
| quote_style: None, | ||||||||||||||
| span: fake_span, | ||||||||||||||
| }, | ||||||||||||||
| data_type: DataType::Text, | ||||||||||||||
| mode: Some(ArgMode::Out), | ||||||||||||||
| default: Some(Expr::Value( | ||||||||||||||
| Value::SingleQuotedString("2".into()).with_empty_span() | ||||||||||||||
| )), | ||||||||||||||
| }, | ||||||||||||||
| ProcedureParam { | ||||||||||||||
| name: Ident { | ||||||||||||||
| value: "c".into(), | ||||||||||||||
| quote_style: None, | ||||||||||||||
| span: fake_span, | ||||||||||||||
| }, | ||||||||||||||
| data_type: DataType::Timestamp(None, TimezoneInfo::None), | ||||||||||||||
| mode: Some(ArgMode::InOut), | ||||||||||||||
| default: Some(Expr::Value(Value::Null.with_empty_span())), | ||||||||||||||
| }, | ||||||||||||||
| ProcedureParam { | ||||||||||||||
| name: Ident { | ||||||||||||||
| value: "d".into(), | ||||||||||||||
| quote_style: None, | ||||||||||||||
| span: fake_span, | ||||||||||||||
| }, | ||||||||||||||
| data_type: DataType::Bool, | ||||||||||||||
| mode: None, | ||||||||||||||
| default: Some(Expr::Value((number("0")).with_empty_span())), | ||||||||||||||
| } | ||||||||||||||
| ]), | ||||||||||||||
| ); | ||||||||||||||
| } | ||||||||||||||
| _ => unreachable!(), | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| #[test] | ||||||||||||||
| fn parse_not_null() { | ||||||||||||||
| let _ = all_dialects().expr_parses_to("x NOT NULL", "x IS NOT NULL"); | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -156,6 +156,7 @@ fn parse_create_procedure() { | |
| }, | ||
| data_type: DataType::Int(None), | ||
| mode: None, | ||
| default: None, | ||
| }, | ||
| ProcedureParam { | ||
| name: Ident { | ||
|
|
@@ -168,6 +169,7 @@ fn parse_create_procedure() { | |
| unit: None | ||
| })), | ||
| mode: None, | ||
| default: None, | ||
| } | ||
| ]), | ||
| name: ObjectName::from(vec![Ident { | ||
|
|
@@ -198,6 +200,12 @@ fn parse_mssql_create_procedure() { | |
| let _ = ms().verified_stmt("CREATE PROCEDURE [foo] AS BEGIN UPDATE bar SET col = 'test'; SELECT [foo] FROM BAR WHERE [FOO] > 10; END"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn parse_mssql_create_procedure_with_parameter_default_value() { | ||
| let sql = r#"CREATE PROCEDURE foo (IN @a INTEGER = 1, OUT @b TEXT = '2', INOUT @c DATETIME = NULL, @d BOOL = 0) AS BEGIN SELECT 1; END"#; | ||
| let _ = ms().verified_stmt(sql); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar comment here that we can inline this scenario into the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done 👍 |
||
|
|
||
| #[test] | ||
| fn parse_create_function() { | ||
| let return_expression_function = "CREATE FUNCTION some_scalar_udf(@foo INT, @bar VARCHAR(256)) RETURNS INT AS BEGIN RETURN 1; END"; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we merge the test into the existing
parse_create_procedure_with_parameter_modes? since they cover the same featureThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I don't understand. A parameter can be in/out and still have/not have a default, right? Are you suggesting like
pub enum ArgMode { In, Out, InOut, + InWithDefault(Expr), + OutWithDefault(Expr), + InOutWithDefault(Expr), }Perhaps I'm not understanding
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah no I meant to only inline this test case into that
parse_create_procedure_with_parameter_modesfunction vs having one function per test caseThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yes, I understand now 👍