-
Notifications
You must be signed in to change notification settings - Fork 605
Add support for table valued functions for SQL Server #1839
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
base: main
Are you sure you want to change the base?
Changes from all commits
54b2829
4793aeb
9f08de0
a54aed6
6346ecc
a27c22c
95f1bc6
999964c
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 | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -48,7 +48,15 @@ pub enum DataType { | |||||||||||||||||||||||||
/// Table type in [PostgreSQL], e.g. CREATE FUNCTION RETURNS TABLE(...). | ||||||||||||||||||||||||||
/// | ||||||||||||||||||||||||||
/// [PostgreSQL]: https://www.postgresql.org/docs/15/sql-createfunction.html | ||||||||||||||||||||||||||
Table(Vec<ColumnDef>), | ||||||||||||||||||||||||||
/// [MsSQL]: https://learn.microsoft.com/en-us/sql/t-sql/statements/create-function-transact-sql?view=sql-server-ver16#c-create-a-multi-statement-table-valued-function | ||||||||||||||||||||||||||
Table(Option<Vec<ColumnDef>>), | ||||||||||||||||||||||||||
/// Table type with a name, e.g. CREATE FUNCTION RETURNS @result TABLE(...). | ||||||||||||||||||||||||||
NamedTable( | ||||||||||||||||||||||||||
/// Table name. | ||||||||||||||||||||||||||
ObjectName, | ||||||||||||||||||||||||||
/// Table columns. | ||||||||||||||||||||||||||
Vec<ColumnDef>, | ||||||||||||||||||||||||||
), | ||||||||||||||||||||||||||
Comment on lines
+54
to
+59
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
we can use an anonymous struct in this manner? |
||||||||||||||||||||||||||
/// Fixed-length character type, e.g. CHARACTER(10). | ||||||||||||||||||||||||||
Character(Option<CharacterLength>), | ||||||||||||||||||||||||||
/// Fixed-length char type, e.g. CHAR(10). | ||||||||||||||||||||||||||
|
@@ -716,7 +724,17 @@ impl fmt::Display for DataType { | |||||||||||||||||||||||||
DataType::Unspecified => Ok(()), | ||||||||||||||||||||||||||
DataType::Trigger => write!(f, "TRIGGER"), | ||||||||||||||||||||||||||
DataType::AnyType => write!(f, "ANY TYPE"), | ||||||||||||||||||||||||||
DataType::Table(fields) => write!(f, "TABLE({})", display_comma_separated(fields)), | ||||||||||||||||||||||||||
DataType::Table(fields) => match fields { | ||||||||||||||||||||||||||
Some(fields) => { | ||||||||||||||||||||||||||
write!(f, "TABLE({})", display_comma_separated(fields)) | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
None => { | ||||||||||||||||||||||||||
write!(f, "TABLE") | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||
DataType::NamedTable(name, fields) => { | ||||||||||||||||||||||||||
write!(f, "{} TABLE ({})", name, display_comma_separated(fields)) | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
DataType::GeometricType(kind) => write!(f, "{}", kind), | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8660,6 +8660,28 @@ pub enum CreateFunctionBody { | |
/// | ||
/// [PostgreSQL]: https://www.postgresql.org/docs/current/sql-createfunction.html | ||
Return(Expr), | ||
|
||
/// Function body expression using the 'AS RETURN' keywords | ||
/// | ||
/// Example: | ||
/// ```sql | ||
/// CREATE FUNCTION myfunc(a INT, b INT) | ||
/// RETURNS TABLE | ||
/// AS RETURN (SELECT a + b AS sum); | ||
/// ``` | ||
/// | ||
/// [MsSql]: https://learn.microsoft.com/en-us/sql/t-sql/statements/create-function-transact-sql | ||
AsReturnSubquery(Expr), | ||
|
||
/// Function body expression using the 'AS RETURN' keywords, with an un-parenthesized SELECT query | ||
/// | ||
/// Example: | ||
/// ```sql | ||
/// CREATE FUNCTION myfunc(a INT, b INT) | ||
/// RETURNS TABLE | ||
/// AS RETURN SELECT a + b AS sum; | ||
/// ``` | ||
AsReturnSelect(Select), | ||
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. maybe we can also include a reference doc link here? |
||
} | ||
|
||
#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -5203,19 +5203,79 @@ impl<'a> Parser<'a> { | |||||||||||||||||
let (name, args) = self.parse_create_function_name_and_params()?; | ||||||||||||||||||
|
||||||||||||||||||
self.expect_keyword(Keyword::RETURNS)?; | ||||||||||||||||||
let return_type = Some(self.parse_data_type()?); | ||||||||||||||||||
|
||||||||||||||||||
self.expect_keyword_is(Keyword::AS)?; | ||||||||||||||||||
let return_table = self.maybe_parse(|p| { | ||||||||||||||||||
let return_table_name = p.parse_identifier()?; | ||||||||||||||||||
let table_column_defs = if p.peek_keyword(Keyword::TABLE) { | ||||||||||||||||||
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. I think we can replace the if/else statement here with |
||||||||||||||||||
match p.parse_data_type()? { | ||||||||||||||||||
DataType::Table(t) => t, | ||||||||||||||||||
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. I think already here we can check that the returned data type is none empty, that would avoid the option invalid case propagating further below (where we have the is_none and is_some checks). e.g. DataType::Table(Some(t)) if !t.is_empty() => t |
||||||||||||||||||
_ => parser_err!( | ||||||||||||||||||
"Expected table data type after TABLE keyword", | ||||||||||||||||||
p.peek_token().span.start | ||||||||||||||||||
)?, | ||||||||||||||||||
} | ||||||||||||||||||
} else { | ||||||||||||||||||
parser_err!( | ||||||||||||||||||
"Expected TABLE keyword after return type", | ||||||||||||||||||
p.peek_token().span.start | ||||||||||||||||||
)? | ||||||||||||||||||
}; | ||||||||||||||||||
|
||||||||||||||||||
let begin_token = self.expect_keyword(Keyword::BEGIN)?; | ||||||||||||||||||
let statements = self.parse_statement_list(&[Keyword::END])?; | ||||||||||||||||||
let end_token = self.expect_keyword(Keyword::END)?; | ||||||||||||||||||
if table_column_defs.is_none() | ||||||||||||||||||
|| table_column_defs.clone().is_some_and(|tcd| tcd.is_empty()) | ||||||||||||||||||
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. hmm is the |
||||||||||||||||||
{ | ||||||||||||||||||
parser_err!( | ||||||||||||||||||
"Expected table column definitions after TABLE keyword", | ||||||||||||||||||
p.peek_token().span.start | ||||||||||||||||||
)? | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
let function_body = Some(CreateFunctionBody::AsBeginEnd(BeginEndStatements { | ||||||||||||||||||
begin_token: AttachedToken(begin_token), | ||||||||||||||||||
statements, | ||||||||||||||||||
end_token: AttachedToken(end_token), | ||||||||||||||||||
})); | ||||||||||||||||||
Ok(DataType::NamedTable( | ||||||||||||||||||
ObjectName(vec![ObjectNamePart::Identifier(return_table_name)]), | ||||||||||||||||||
table_column_defs.clone().unwrap(), | ||||||||||||||||||
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. can we return an error instead of the unwrap here? 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. similarly here, is the |
||||||||||||||||||
)) | ||||||||||||||||||
})?; | ||||||||||||||||||
|
||||||||||||||||||
let return_type = if return_table.is_some() { | ||||||||||||||||||
return_table | ||||||||||||||||||
} else { | ||||||||||||||||||
Some(self.parse_data_type()?) | ||||||||||||||||||
}; | ||||||||||||||||||
Comment on lines
+5239
to
+5243
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. Could this be included in the 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. Hm, I don't see how that would be done. The use of maybe_parse is to get the table name & other parts, but if we don't have that then we should rewind and just parse a typical data type. Do you mean like having another maybe_parse inside the first maybe_parse? 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. Ah indeed, in this case its one or the other, so should be fine to leave as is. |
||||||||||||||||||
|
||||||||||||||||||
let _ = self.parse_keyword(Keyword::AS); | ||||||||||||||||||
|
||||||||||||||||||
let function_body = if self.peek_keyword(Keyword::BEGIN) { | ||||||||||||||||||
let begin_token = self.expect_keyword(Keyword::BEGIN)?; | ||||||||||||||||||
let statements = self.parse_statement_list(&[Keyword::END])?; | ||||||||||||||||||
let end_token = self.expect_keyword(Keyword::END)?; | ||||||||||||||||||
|
||||||||||||||||||
Some(CreateFunctionBody::AsBeginEnd(BeginEndStatements { | ||||||||||||||||||
begin_token: AttachedToken(begin_token), | ||||||||||||||||||
statements, | ||||||||||||||||||
end_token: AttachedToken(end_token), | ||||||||||||||||||
})) | ||||||||||||||||||
} else if self.parse_keyword(Keyword::RETURN) { | ||||||||||||||||||
if self.peek_token() == Token::LParen { | ||||||||||||||||||
let expr = self.parse_expr()?; | ||||||||||||||||||
if !matches!(expr, Expr::Subquery(_)) { | ||||||||||||||||||
parser_err!( | ||||||||||||||||||
"Expected a subquery after RETURN", | ||||||||||||||||||
self.peek_token().span.start | ||||||||||||||||||
)? | ||||||||||||||||||
} | ||||||||||||||||||
Some(CreateFunctionBody::AsReturnSubquery(expr)) | ||||||||||||||||||
Comment on lines
+5260
to
+5266
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
thinking since a subquery is already an expr, we can be more permissive here? |
||||||||||||||||||
} else if self.peek_keyword(Keyword::SELECT) { | ||||||||||||||||||
let select = self.parse_select()?; | ||||||||||||||||||
Some(CreateFunctionBody::AsReturnSelect(select)) | ||||||||||||||||||
} else { | ||||||||||||||||||
parser_err!( | ||||||||||||||||||
"Expected a subquery (or bare SELECT statement) after RETURN", | ||||||||||||||||||
self.peek_token().span.start | ||||||||||||||||||
)? | ||||||||||||||||||
} | ||||||||||||||||||
} else { | ||||||||||||||||||
parser_err!("Unparsable function body", self.peek_token().span.start)? | ||||||||||||||||||
}; | ||||||||||||||||||
|
||||||||||||||||||
Ok(Statement::CreateFunction(CreateFunction { | ||||||||||||||||||
or_alter, | ||||||||||||||||||
|
@@ -9766,8 +9826,12 @@ impl<'a> Parser<'a> { | |||||||||||||||||
Ok(DataType::AnyType) | ||||||||||||||||||
} | ||||||||||||||||||
Keyword::TABLE => { | ||||||||||||||||||
let columns = self.parse_returns_table_columns()?; | ||||||||||||||||||
Ok(DataType::Table(columns)) | ||||||||||||||||||
if self.peek_token() != Token::LParen { | ||||||||||||||||||
Ok(DataType::Table(None)) | ||||||||||||||||||
} else { | ||||||||||||||||||
let columns = self.parse_returns_table_columns()?; | ||||||||||||||||||
Ok(DataType::Table(Some(columns))) | ||||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
Keyword::SIGNED => { | ||||||||||||||||||
if self.parse_keyword(Keyword::INTEGER) { | ||||||||||||||||||
|
@@ -9808,13 +9872,7 @@ impl<'a> Parser<'a> { | |||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
fn parse_returns_table_column(&mut self) -> Result<ColumnDef, ParserError> { | ||||||||||||||||||
let name = self.parse_identifier()?; | ||||||||||||||||||
let data_type = self.parse_data_type()?; | ||||||||||||||||||
Ok(ColumnDef { | ||||||||||||||||||
name, | ||||||||||||||||||
data_type, | ||||||||||||||||||
options: Vec::new(), // No constraints expected here | ||||||||||||||||||
}) | ||||||||||||||||||
self.parse_column_def() | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
fn parse_returns_table_columns(&mut self) -> Result<Vec<ColumnDef>, ParserError> { | ||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -254,6 +254,12 @@ fn parse_create_function() { | |
"; | ||
let _ = ms().verified_stmt(multi_statement_function); | ||
|
||
let multi_statement_function_without_as = multi_statement_function.replace(" AS", ""); | ||
let _ = ms().one_statement_parses_to( | ||
&multi_statement_function_without_as, | ||
multi_statement_function, | ||
); | ||
|
||
let create_function_with_conditional = "\ | ||
CREATE FUNCTION some_scalar_udf() \ | ||
RETURNS INT \ | ||
|
@@ -288,6 +294,58 @@ fn parse_create_function() { | |
END\ | ||
"; | ||
let _ = ms().verified_stmt(create_function_with_return_expression); | ||
|
||
let create_inline_table_value_function = "\ | ||
CREATE FUNCTION some_inline_tvf(@foo INT, @bar VARCHAR(256)) \ | ||
RETURNS TABLE \ | ||
AS \ | ||
RETURN (SELECT 1 AS col_1)\ | ||
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. Parentheses are optional for inline tvf 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. I added support for that syntax & added a new test case example 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.
|
||
"; | ||
let _ = ms().verified_stmt(create_inline_table_value_function); | ||
|
||
let create_inline_table_value_function_without_parentheses = "\ | ||
CREATE FUNCTION some_inline_tvf(@foo INT, @bar VARCHAR(256)) \ | ||
RETURNS TABLE \ | ||
AS \ | ||
RETURN SELECT 1 AS col_1\ | ||
"; | ||
let _ = ms().verified_stmt(create_inline_table_value_function_without_parentheses); | ||
|
||
let create_inline_table_value_function_without_as = | ||
create_inline_table_value_function.replace(" AS", ""); | ||
let _ = ms().one_statement_parses_to( | ||
&create_inline_table_value_function_without_as, | ||
create_inline_table_value_function, | ||
); | ||
|
||
let create_multi_statement_table_value_function = "\ | ||
CREATE FUNCTION some_multi_statement_tvf(@foo INT, @bar VARCHAR(256)) \ | ||
RETURNS @t TABLE (col_1 INT) \ | ||
AS \ | ||
BEGIN \ | ||
INSERT INTO @t SELECT 1; \ | ||
RETURN; \ | ||
END\ | ||
"; | ||
let _ = ms().verified_stmt(create_multi_statement_table_value_function); | ||
|
||
let create_multi_statement_table_value_function_without_as = | ||
create_multi_statement_table_value_function.replace(" AS", ""); | ||
let _ = ms().one_statement_parses_to( | ||
&create_multi_statement_table_value_function_without_as, | ||
create_multi_statement_table_value_function, | ||
); | ||
|
||
let create_multi_statement_table_value_function_with_constraints = "\ | ||
CREATE FUNCTION some_multi_statement_tvf(@foo INT, @bar VARCHAR(256)) \ | ||
RETURNS @t TABLE (col_1 INT NOT NULL) \ | ||
AS \ | ||
BEGIN \ | ||
INSERT INTO @t SELECT 1; \ | ||
RETURN @t; \ | ||
END\ | ||
"; | ||
let _ = ms().verified_stmt(create_multi_statement_table_value_function_with_constraints); | ||
} | ||
|
||
#[test] | ||
|
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.
Could we add a link to the docs that support
NamedTable
variant?