Skip to content

Commit

Permalink
nom-sql: Parse arbitrary exprs in args to group_concat
Browse files Browse the repository at this point in the history
I'm pretty sure the previous behavior of only parsing columns is a
legacy accident from before we had a more flexible expression AST -
regardless, MySQL accepts any expr here, so we should as well.

Change-Id: I505a780da514d216abdd97cea63c55980a5e6c8b
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/5161
Tested-by: Buildkite CI
Reviewed-by: Dan Wilbanks <dan@readyset.io>
  • Loading branch information
glittershark committed Jun 14, 2023
1 parent 529ba86 commit 6f16afe
Showing 1 changed file with 6 additions and 11 deletions.
17 changes: 6 additions & 11 deletions nom-sql/src/common.rs
Expand Up @@ -417,13 +417,8 @@ fn group_concat_fx_helper(

fn group_concat_fx(
dialect: Dialect,
) -> impl Fn(LocatedSpan<&[u8]>) -> NomSqlResult<&[u8], (Column, Option<String>)> {
move |i| {
pair(
column_identifier_no_alias(dialect),
opt(group_concat_fx_helper(dialect)),
)(i)
}
) -> impl Fn(LocatedSpan<&[u8]>) -> NomSqlResult<&[u8], (Expr, Option<String>)> {
move |i| pair(expression(dialect), opt(group_concat_fx_helper(dialect)))(i)
}

fn agg_fx_args(
Expand Down Expand Up @@ -576,8 +571,8 @@ pub fn function_expr(
),
),
),
|(col, separator)| FunctionExpr::GroupConcat {
expr: Box::new(Expr::Column(col)),
|(expr, separator)| FunctionExpr::GroupConcat {
expr: Box::new(expr),
separator,
},
),
Expand Down Expand Up @@ -896,9 +891,9 @@ mod tests {
assert_eq!(res.unwrap().1, expected);

assert_eq!(
test_parse!(function_expr(Dialect::MySQL), b"group_concat(a)"),
test_parse!(function_expr(Dialect::MySQL), b"group_concat('a')"),
FunctionExpr::GroupConcat {
expr: Box::new(Expr::Column("a".into())),
expr: Box::new(Expr::Literal("a".into())),
separator: None
}
);
Expand Down

0 comments on commit 6f16afe

Please sign in to comment.