Skip to content

Commit 301f78f

Browse files
committed
nom-sql: Fix GROUP BY parsing for Postgres
Postgres allows you to identify columns in a `GROUP BY` clause by their ordinal position in the column list of a query; however our parser was written with the assumption that this was only possible for MySQL. This commit updates our parser to support column indices in the `GROUP BY` clause for both MySQL *and* Postgres. Release-Note-Core: Fixed an issue where Readyset was not correctly handling entries in the `GROUP BY` clause that identified columns by their position in the column list of the query Change-Id: Ib84789e903ca8b7137b780ee1e8e791439dab1ae Reviewed-on: https://gerrit.readyset.name/c/readyset/+/6980 Tested-by: Buildkite CI Reviewed-by: Luke Osborne <[email protected]>
1 parent 048fd4b commit 301f78f

File tree

2 files changed

+63
-14
lines changed

2 files changed

+63
-14
lines changed
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
statement ok
2+
create table t (
3+
id int,
4+
val int
5+
);
6+
7+
statement ok
8+
insert into t (id, val) values
9+
(1, 1),
10+
(2, 1),
11+
(3, 2),
12+
(4, 2);
13+
14+
query II nosort
15+
select id, val from t order by 1
16+
----
17+
1
18+
1
19+
2
20+
1
21+
3
22+
2
23+
4
24+
2
25+
26+
27+
query II nosort
28+
select val, id from t order by 2
29+
----
30+
1
31+
1
32+
1
33+
2
34+
2
35+
3
36+
2
37+
4
38+
39+
query II rowsort
40+
select max(id), val from t group by 2
41+
----
42+
2
43+
1
44+
4
45+
2
46+
47+
48+
query II nosort
49+
select max(id), val from t group by 2 order by 2 desc
50+
----
51+
4
52+
2
53+
2
54+
1

nom-sql/src/common.rs

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -827,21 +827,16 @@ pub fn field_reference(
827827
dialect: Dialect,
828828
) -> impl Fn(LocatedSpan<&[u8]>) -> NomSqlResult<&[u8], FieldReference> {
829829
move |i| {
830-
match dialect {
831-
Dialect::PostgreSQL => map(expression(dialect), FieldReference::Expr)(i),
832-
// Only MySQL supports numeric field references (postgresql considers them integer
833-
// literals, I'm pretty sure)
834-
Dialect::MySQL => alt((
835-
map(
836-
map_res(
837-
map_res(digit1, |i: LocatedSpan<&[u8]>| str::from_utf8(&i)),
838-
u64::from_str,
839-
),
840-
FieldReference::Numeric,
830+
alt((
831+
map(
832+
map_res(
833+
map_res(digit1, |i: LocatedSpan<&[u8]>| str::from_utf8(&i)),
834+
u64::from_str,
841835
),
842-
map(expression(dialect), FieldReference::Expr),
843-
))(i),
844-
}
836+
FieldReference::Numeric,
837+
),
838+
map(expression(dialect), FieldReference::Expr),
839+
))(i)
845840
}
846841
}
847842

0 commit comments

Comments
 (0)