Skip to content

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
22 changes: 20 additions & 2 deletions src/ast/data_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(...).
Copy link
Contributor

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?

NamedTable(
/// Table name.
ObjectName,
/// Table columns.
Vec<ColumnDef>,
),
Comment on lines +54 to +59
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
NamedTable(
/// Table name.
ObjectName,
/// Table columns.
Vec<ColumnDef>,
),
NamedTable {
/// Table name.
table: ObjectName,
/// Table columns.
columns: Vec<ColumnDef>,
},

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).
Expand Down Expand Up @@ -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),
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/ast/ddl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2313,6 +2313,12 @@ impl fmt::Display for CreateFunction {
if let Some(CreateFunctionBody::Return(function_body)) = &self.function_body {
write!(f, " RETURN {function_body}")?;
}
if let Some(CreateFunctionBody::AsReturnSubquery(function_body)) = &self.function_body {
write!(f, " AS RETURN {function_body}")?;
}
if let Some(CreateFunctionBody::AsReturnSelect(function_body)) = &self.function_body {
write!(f, " AS RETURN {function_body}")?;
}
if let Some(using) = &self.using {
write!(f, " {using}")?;
}
Expand Down
22 changes: 22 additions & 0 deletions src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Copy link
Contributor

Choose a reason for hiding this comment

The 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)]
Expand Down
96 changes: 77 additions & 19 deletions src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can replace the if/else statement here with self.expect_keyword(Keyword::TABLE)

match p.parse_data_type()? {
DataType::Table(t) => t,
Copy link
Contributor

Choose a reason for hiding this comment

The 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())
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm is the clone() here necessary?

{
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(),
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 return an error instead of the unwrap here?

Copy link
Contributor

Choose a reason for hiding this comment

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

similarly here, is the clone necessary?

))
})?;

let return_type = if return_table.is_some() {
return_table
} else {
Some(self.parse_data_type()?)
};
Comment on lines +5239 to +5243
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be included in the maybe_parse() logic above? such that the logic returns both the return table and the datatype. based on this condition, they seem to go hand in hand?

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 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?

Copy link
Contributor

Choose a reason for hiding this comment

The 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
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
if !matches!(expr, Expr::Subquery(_)) {
parser_err!(
"Expected a subquery after RETURN",
self.peek_token().span.start
)?
}
Some(CreateFunctionBody::AsReturnSubquery(expr))
Some(CreateFunctionBody::AsReturnExpr(expr))

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,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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> {
Expand Down
58 changes: 58 additions & 0 deletions tests/sqlparser_mssql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down Expand Up @@ -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)\
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Parentheses are optional for inline tvf return queries, although I think the subquery expr expects/requires them currently.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added support for that syntax & added a new test case example

Copy link
Contributor Author

@aharpervc aharpervc May 7, 2025

Choose a reason for hiding this comment

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

UNION is also supported in this syntax but not in this current approach due to using parse_select, which is restricted. I'm comfortable leaving that for later

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