Skip to content

Commit

Permalink
nom-sql: Allow spaces between and around aggregate args
Browse files Browse the repository at this point in the history
Allow parsing spaces before, between, and around the parentheses used
for the arguments to all aggregate functions - this allows us to eg
successfully parse `count(*)`, which both mysql and postgresql accept.

Fixes: REA-2884
Change-Id: Ia68c313289bdfaeae880ae226f2a7ee175594141
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/5159
Tested-by: Buildkite CI
Reviewed-by: Dan Wilbanks <dan@readyset.io>
  • Loading branch information
glittershark committed Jun 14, 2023
1 parent 2649e2f commit 529ba86
Showing 1 changed file with 62 additions and 3 deletions.
65 changes: 62 additions & 3 deletions nom-sql/src/common.rs
Expand Up @@ -429,7 +429,10 @@ fn group_concat_fx(
fn agg_fx_args(
dialect: Dialect,
) -> impl Fn(LocatedSpan<&[u8]>) -> NomSqlResult<&[u8], (Expr, bool)> {
move |i| delimited(tag("("), agg_function_arguments(dialect), tag(")"))(i)
move |i| {
let (i, _) = whitespace0(i)?;
delimited(tag("("), agg_function_arguments(dialect), tag(")"))(i)
}
}

fn delim_fx_args(
Expand Down Expand Up @@ -524,7 +527,18 @@ pub fn function_expr(
) -> impl Fn(LocatedSpan<&[u8]>) -> NomSqlResult<&[u8], FunctionExpr> {
move |i| {
alt((
map(tag_no_case("count(*)"), |_| FunctionExpr::CountStar),
map(
tuple((
tag_no_case("count"),
whitespace0,
tag("("),
whitespace0,
tag("*"),
whitespace0,
tag(")"),
)),
|_| FunctionExpr::CountStar,
),
map(
preceded(tag_no_case("count"), agg_fx_args(dialect)),
|args| FunctionExpr::Count {
Expand Down Expand Up @@ -553,7 +567,14 @@ pub fn function_expr(
map(
preceded(
tag_no_case("group_concat"),
delimited(tag("("), group_concat_fx(dialect), tag(")")),
preceded(
whitespace0,
delimited(
terminated(tag("("), whitespace0),
group_concat_fx(dialect),
preceded(whitespace0, tag(")")),
),
),
),
|(col, separator)| FunctionExpr::GroupConcat {
expr: Box::new(Expr::Column(col)),
Expand Down Expand Up @@ -873,6 +894,28 @@ mod tests {
};
let res = to_nom_result(function_expr(Dialect::MySQL)(LocatedSpan::new(qs)));
assert_eq!(res.unwrap().1, expected);

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

#[test]
Expand Down Expand Up @@ -1031,6 +1074,22 @@ mod tests {
);
}

#[test]
fn count_star() {
assert_eq!(
test_parse!(function_expr(Dialect::MySQL), b"count(*)"),
FunctionExpr::CountStar
);
assert_eq!(
test_parse!(function_expr(Dialect::MySQL), b"count (*)"),
FunctionExpr::CountStar
);
assert_eq!(
test_parse!(function_expr(Dialect::MySQL), b"count ( * )"),
FunctionExpr::CountStar
);
}

mod mysql {
use super::*;

Expand Down

0 comments on commit 529ba86

Please sign in to comment.