Skip to content

Commit a612d1b

Browse files
sbdchdsbdchd
committed
parser: fix select precedence for trailing clauses
1 parent 274ba82 commit a612d1b

File tree

3 files changed

+72
-34
lines changed

3 files changed

+72
-34
lines changed

‎crates/squawk_parser/src/grammar.rs‎

Lines changed: 51 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,11 @@ fn array_expr(p: &mut Parser<'_>, m: Option<Marker>) -> CompletedMarker {
5555
R_BRACK
5656
};
5757
while !p.at(EOF) && !p.at(closing) {
58-
if p.at_ts(SELECT_FIRST) && (select(p, None).is_none() || p.at(EOF) || p.at(closing)) {
58+
if p.at_ts(SELECT_FIRST)
59+
&& (select(p, None, &SelectRestrictions::default()).is_none()
60+
|| p.at(EOF)
61+
|| p.at(closing))
62+
{
5963
break;
6064
}
6165
if expr(p).is_none() {
@@ -73,6 +77,18 @@ fn array_expr(p: &mut Parser<'_>, m: Option<Marker>) -> CompletedMarker {
7377
m.complete(p, ARRAY_EXPR)
7478
}
7579

80+
struct SelectRestrictions {
81+
trailing_clauses: bool,
82+
}
83+
84+
impl Default for SelectRestrictions {
85+
fn default() -> Self {
86+
Self {
87+
trailing_clauses: true,
88+
}
89+
}
90+
}
91+
7692
fn opt_paren_select(p: &mut Parser<'_>) -> Option<CompletedMarker> {
7793
let m = p.start();
7894
if !p.eat(L_PAREN) {
@@ -83,7 +99,11 @@ fn opt_paren_select(p: &mut Parser<'_>) -> Option<CompletedMarker> {
8399
// saw_expr = true;
84100
// we want to check for select stuff before we get the the expr stuff maybe? Although select is an expr so maybe fine? but it's not prefix or postfix so maybe right here is good?
85101
//
86-
if p.at_ts(SELECT_FIRST) && (select(p, None).is_none() || p.at(EOF) || p.at(R_PAREN)) {
102+
if p.at_ts(SELECT_FIRST)
103+
&& (select(p, None, &SelectRestrictions::default()).is_none()
104+
|| p.at(EOF)
105+
|| p.at(R_PAREN))
106+
{
87107
break;
88108
}
89109
if opt_paren_select(p).is_none() {
@@ -121,7 +141,11 @@ fn tuple_expr(p: &mut Parser<'_>) -> CompletedMarker {
121141
saw_expr = true;
122142
// we want to check for select stuff before we get the the expr stuff maybe? Although select is an expr so maybe fine? but it's not prefix or postfix so maybe right here is good?
123143
//
124-
if p.at_ts(SELECT_FIRST) && (select(p, None).is_none() || p.at(EOF) || p.at(R_PAREN)) {
144+
if p.at_ts(SELECT_FIRST)
145+
&& (select(p, None, &SelectRestrictions::default()).is_none()
146+
|| p.at(EOF)
147+
|| p.at(R_PAREN))
148+
{
125149
break;
126150
}
127151
if expr(p).is_none() {
@@ -691,7 +715,10 @@ fn json_array_fn_arg_list(p: &mut Parser<'_>) {
691715
// 1, 2, 3, 4
692716
while !p.at(EOF) && !p.at(R_PAREN) && !p.at(RETURNING_KW) {
693717
if p.at_ts(SELECT_FIRST) {
694-
if select(p, None).is_none() || p.at(EOF) || p.at(R_PAREN) {
718+
if select(p, None, &SelectRestrictions::default()).is_none()
719+
|| p.at(EOF)
720+
|| p.at(R_PAREN)
721+
{
695722
break;
696723
}
697724
opt_json_format_clause(p);
@@ -732,7 +759,7 @@ fn some_any_all_fn(p: &mut Parser<'_>) -> CompletedMarker {
732759
// args
733760
p.expect(L_PAREN);
734761
if p.at_ts(SELECT_FIRST) {
735-
select(p, None);
762+
select(p, None, &SelectRestrictions::default());
736763
} else {
737764
if expr(p).is_none() {
738765
p.error("expected expression or select");
@@ -826,7 +853,7 @@ fn exists_fn(p: &mut Parser<'_>) -> CompletedMarker {
826853
assert!(p.at(EXISTS_KW));
827854
custom_fn(p, EXISTS_KW, |p| {
828855
if p.at_ts(SELECT_FIRST) {
829-
select(p, None);
856+
select(p, None, &SelectRestrictions::default());
830857
} else {
831858
p.error("expected select")
832859
}
@@ -2457,18 +2484,25 @@ fn compound_select(p: &mut Parser<'_>, cm: CompletedMarker) -> CompletedMarker {
24572484
opt_paren_select(p);
24582485
} else {
24592486
if p.at_ts(SELECT_FIRST) {
2460-
select(p, None);
2487+
select(
2488+
p,
2489+
None,
2490+
&SelectRestrictions {
2491+
trailing_clauses: false,
2492+
},
2493+
);
24612494
} else {
24622495
p.error("expected start of a select statement")
24632496
}
24642497
}
2498+
select_trailing_clauses(p);
24652499
m.complete(p, COMPOUND_SELECT)
24662500
}
24672501

24682502
// error recovery:
24692503
// - <https://youtu.be/0HlrqwLjCxA?feature=shared&t=2172>
24702504
/// <https://www.postgresql.org/docs/17/sql-select.html>
2471-
fn select(p: &mut Parser, m: Option<Marker>) -> Option<CompletedMarker> {
2505+
fn select(p: &mut Parser, m: Option<Marker>, r: &SelectRestrictions) -> Option<CompletedMarker> {
24722506
assert!(p.at_ts(SELECT_FIRST));
24732507
let m = m.unwrap_or_else(|| p.start());
24742508

@@ -2506,7 +2540,9 @@ fn select(p: &mut Parser, m: Option<Marker>) -> Option<CompletedMarker> {
25062540
let cm = m.complete(p, SELECT);
25072541
return Some(compound_select(p, cm));
25082542
}
2509-
select_trailing_clauses(p);
2543+
if r.trailing_clauses {
2544+
select_trailing_clauses(p);
2545+
}
25102546
Some(m.complete(p, out_kind))
25112547
}
25122548

@@ -5339,7 +5375,7 @@ fn stmt(p: &mut Parser, r: &StmtRestrictions) -> Option<CompletedMarker> {
53395375
(ROLLBACK_KW, _) => Some(rollback(p)),
53405376
(SAVEPOINT_KW, _) => Some(savepoint(p)),
53415377
(SECURITY_KW, LABEL_KW) => Some(security_label(p)),
5342-
(SELECT_KW | TABLE_KW | VALUES_KW, _) => select(p, None),
5378+
(SELECT_KW | TABLE_KW | VALUES_KW, _) => select(p, None, &SelectRestrictions::default()),
53435379
(SET_KW, CONSTRAINTS_KW) => Some(set_constraints(p)),
53445380
(SET_KW, LOCAL_KW) => match p.nth(2) {
53455381
ROLE_KW => Some(set_role(p)),
@@ -12006,7 +12042,9 @@ fn create_schema(p: &mut Parser<'_>) -> CompletedMarker {
1200612042

1200712043
fn query(p: &mut Parser<'_>) {
1200812044
// TODO: this needs to be more general
12009-
if (!p.at_ts(SELECT_FIRST) || select(p, None).is_none()) && opt_paren_select(p).is_none() {
12045+
if (!p.at_ts(SELECT_FIRST) || select(p, None, &SelectRestrictions::default()).is_none())
12046+
&& opt_paren_select(p).is_none()
12047+
{
1201012048
p.error("expected select stmt")
1201112049
}
1201212050
}
@@ -12127,7 +12165,7 @@ fn set_clause(p: &mut Parser<'_>) {
1212712165
if p.eat(L_PAREN) {
1212812166
// ( sub-SELECT )
1212912167
if p.at(SELECT_KW) && !found_row {
12130-
if select(p, None).is_none() {
12168+
if select(p, None, &SelectRestrictions::default()).is_none() {
1213112169
p.error("expected sub-SELECT");
1213212170
}
1213312171
} else {
@@ -12222,7 +12260,7 @@ fn with(p: &mut Parser<'_>, m: Option<Marker>) -> Option<CompletedMarker> {
1222212260
with_query_clause(p);
1222312261
match p.current() {
1222412262
DELETE_KW => Some(delete(p, Some(m))),
12225-
SELECT_KW | TABLE_KW | VALUES_KW => select(p, Some(m)),
12263+
SELECT_KW | TABLE_KW | VALUES_KW => select(p, Some(m), &SelectRestrictions::default()),
1222612264
INSERT_KW => Some(insert(p, Some(m))),
1222712265
UPDATE_KW => Some(update(p, Some(m))),
1222812266
MERGE_KW => Some(merge(p, Some(m))),

‎crates/squawk_parser/tests/snapshots/tests__precedence_ok.snap‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -95,14 +95,14 @@ SOURCE_FILE
9595
TARGET
9696
NAME_REF
9797
IDENT "bar"
98+
WHITESPACE " "
99+
ORDER_BY_CLAUSE
100+
ORDER_KW "ORDER"
98101
WHITESPACE " "
99-
ORDER_BY_CLAUSE
100-
ORDER_KW "ORDER"
101-
WHITESPACE " "
102-
BY_KW "BY"
103-
WHITESPACE " "
104-
NAME_REF
105-
IDENT "baz"
102+
BY_KW "BY"
103+
WHITESPACE " "
104+
NAME_REF
105+
IDENT "baz"
106106
SEMICOLON ";"
107107
WHITESPACE "\n"
108108
COMMENT "-- equal to:"

‎crates/squawk_parser/tests/snapshots/tests__select_compound_union_select_ok.snap‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -236,22 +236,22 @@ SOURCE_FILE
236236
WHITESPACE " "
237237
LITERAL
238238
STRING "'nl-NL'"
239+
WHITESPACE " "
240+
ORDER_BY_CLAUSE
241+
ORDER_KW "ORDER"
239242
WHITESPACE " "
240-
ORDER_BY_CLAUSE
241-
ORDER_KW "ORDER"
242-
WHITESPACE " "
243-
BY_KW "BY"
244-
WHITESPACE " "
245-
NAME_REF
246-
IDENT "\"id\""
247-
WHITESPACE " "
248-
ASC_KW "ASC"
243+
BY_KW "BY"
249244
WHITESPACE " "
250-
LIMIT_CLAUSE
251-
LIMIT_KW "LIMIT"
252-
WHITESPACE " "
253-
LITERAL
254-
INT_NUMBER "1"
245+
NAME_REF
246+
IDENT "\"id\""
247+
WHITESPACE " "
248+
ASC_KW "ASC"
249+
WHITESPACE " "
250+
LIMIT_CLAUSE
251+
LIMIT_KW "LIMIT"
252+
WHITESPACE " "
253+
LITERAL
254+
INT_NUMBER "1"
255255
WHITESPACE "\n"
256256
R_PAREN ")"
257257
WHITESPACE " "

0 commit comments

Comments
 (0)