Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/ast/ddl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1458,12 +1458,19 @@ pub struct ProcedureParam {
pub name: Ident,
pub data_type: DataType,
pub mode: Option<ArgMode>,
pub default: Option<Expr>,
}

impl fmt::Display for ProcedureParam {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
if let Some(mode) = &self.mode {
write!(f, "{mode} {} {}", self.name, self.data_type)
if let Some(default) = &self.default {
write!(f, "{mode} {} {} = {}", self.name, self.data_type, default)
} else {
write!(f, "{mode} {} {}", self.name, self.data_type)
}
} else if let Some(default) = &self.default {
write!(f, "{} {} = {}", self.name, self.data_type, default)
} else {
write!(f, "{} {}", self.name, self.data_type)
}
Expand Down
7 changes: 7 additions & 0 deletions src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7897,10 +7897,17 @@ impl<'a> Parser<'a> {
};
let name = self.parse_identifier()?;
let data_type = self.parse_data_type()?;
let default = if self.consume_token(&Token::Eq) {
Some(self.parse_expr()?)
} else {
None
};

Ok(ProcedureParam {
name,
data_type,
mode,
default,
})
}

Expand Down
80 changes: 76 additions & 4 deletions tests/sqlparser_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -16524,7 +16527,8 @@ fn parse_create_procedure_with_parameter_modes() {
span: fake_span,
},
data_type: DataType::Bool,
mode: None
mode: None,
default: None,
},
])
);
Expand All @@ -16533,6 +16537,74 @@ fn parse_create_procedure_with_parameter_modes() {
}
}

#[test]
fn create_procedure_with_parameter_default_value() {
Copy link
Contributor

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 feature

Copy link
Contributor Author

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

Copy link
Contributor

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_modes function vs having one function per test case

Copy link
Contributor Author

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 👍

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,
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
name: Ident {
value: "a".into(),
quote_style: None,
span: fake_span,
},
name: Ident::new("a"),

I think this can be simplified? that would let us skip introducing the fake span as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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");
Expand Down
8 changes: 8 additions & 0 deletions tests/sqlparser_mssql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ fn parse_create_procedure() {
},
data_type: DataType::Int(None),
mode: None,
default: None,
},
ProcedureParam {
name: Ident {
Expand All @@ -168,6 +169,7 @@ fn parse_create_procedure() {
unit: None
})),
mode: None,
default: None,
}
]),
name: ObjectName::from(vec![Ident {
Expand Down Expand Up @@ -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);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar comment here that we can inline this scenario into the parse_mssql_create_procedure function above

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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";
Expand Down