Skip to content

Commit 39a0f04

Browse files
Parser: fix exponential parse time on table-factor paren chains
`parse_table_factor` speculatively parses `(...)` as a derived table; on failure it rewinds and falls through to `parse_table_and_joins`, which recurses back into `parse_table_factor` and re-attempts the same speculative parse at every deeper paren. Both arms walk the remaining chain, so on inputs like `SELECT 1 FROM ((((...` work doubles per level. Caching the position at which the speculative arm failed short-circuits the second descent. Measured on `GenericDialect` with `with_recursion_limit(256)`, release build: | N | Before | After | |----|---------|--------| | 10 | 20 ms | 57 us | | 20 | 820 ms | 119 us | | 25 | 2.8 s | 281 us | | 30 | 7.9 s | 345 us |
1 parent b376022 commit 39a0f04

3 files changed

Lines changed: 88 additions & 5 deletions

File tree

sqlparser_bench/benches/sqlparser_bench.rs

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,34 @@ fn parse_prefix_case_chain(c: &mut Criterion) {
245245
group.finish();
246246
}
247247

248+
/// Benchmark parsing pathological paren chains that previously caused 2^N
249+
/// work in `parse_table_factor`. The input `SELECT 1 FROM ((((...` rejects
250+
/// at EOF, which used to force exponential backtracking through the chain.
251+
fn parse_table_factor_paren_chain(c: &mut Criterion) {
252+
let mut group = c.benchmark_group("parse_table_factor_paren_chain");
253+
let dialect = GenericDialect {};
254+
255+
for &n in &[10usize, 20, 30] {
256+
let mut sql = String::from("SELECT 1 ");
257+
for _ in 0..5 {
258+
sql.push_str("FROM ");
259+
sql.push_str(&"(".repeat(n));
260+
sql.push(' ');
261+
}
262+
263+
group.bench_function(format!("chain_{n}"), |b| {
264+
b.iter(|| {
265+
let _ = Parser::new(&dialect)
266+
.with_recursion_limit(256)
267+
.try_with_sql(std::hint::black_box(&sql))
268+
.and_then(|mut p| p.parse_statements());
269+
});
270+
});
271+
}
272+
273+
group.finish();
274+
}
275+
248276
criterion_group!(
249277
benches,
250278
basic_queries,
@@ -253,6 +281,7 @@ criterion_group!(
253281
parse_compound_chain,
254282
parse_compound_keyword_chain,
255283
parse_prefix_keyword_call_chain,
256-
parse_prefix_case_chain
284+
parse_prefix_case_chain,
285+
parse_table_factor_paren_chain
257286
);
258287
criterion_main!(benches);

src/parser/mod.rs

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
#[cfg(not(feature = "std"))]
1616
use alloc::{
1717
boxed::Box,
18-
collections::BTreeMap,
18+
collections::{BTreeMap, BTreeSet},
1919
format,
2020
string::{String, ToString},
2121
vec,
@@ -26,7 +26,7 @@ use core::{
2626
str::FromStr,
2727
};
2828
#[cfg(feature = "std")]
29-
use std::collections::BTreeMap;
29+
use std::collections::{BTreeMap, BTreeSet};
3030

3131
use helpers::attached_token::AttachedToken;
3232

@@ -369,6 +369,10 @@ pub struct Parser<'a> {
369369
/// Cached failures from the speculative reserved-word prefix arm. See
370370
/// [`Parser::parse_prefix`] for the 2^N patterns this guards.
371371
failed_reserved_word_prefix_positions: BTreeMap<usize, ExprPrefixError>,
372+
/// Cached failures from the speculative derived-table arm of
373+
/// `parse_table_factor`. See [`Parser::parse_table_factor`] for the 2^N
374+
/// pattern this guards.
375+
failed_derived_table_factor_positions: BTreeSet<usize>,
372376
}
373377

374378
/// Copy marker for a [`ParserError`] cached by the `parse_prefix` failure
@@ -414,6 +418,7 @@ impl<'a> Parser<'a> {
414418
options: ParserOptions::new().with_trailing_commas(dialect.supports_trailing_commas()),
415419
failed_prefix_positions: BTreeMap::new(),
416420
failed_reserved_word_prefix_positions: BTreeMap::new(),
421+
failed_derived_table_factor_positions: BTreeSet::new(),
417422
}
418423
}
419424

@@ -477,6 +482,7 @@ impl<'a> Parser<'a> {
477482
self.index = 0;
478483
self.failed_prefix_positions.clear();
479484
self.failed_reserved_word_prefix_positions.clear();
485+
self.failed_derived_table_factor_positions.clear();
480486
self
481487
}
482488

@@ -16172,9 +16178,26 @@ impl<'a> Parser<'a> {
1617216178
// `parse_derived_table_factor` below will return success after parsing the
1617316179
// subquery, followed by the closing ')', and the alias of the derived table.
1617416180
// In the example above this is case (3).
16175-
if let Some(mut table) =
16176-
self.maybe_parse(|parser| parser.parse_derived_table_factor(NotLateral))?
16181+
//
16182+
// Memoize failures to break the 2^N work on inputs like
16183+
// `FROM ((((...`, where the nested-join fallback recurses back into
16184+
// `parse_table_factor` and re-attempts the same speculative parse.
16185+
let derived_pos = self.index;
16186+
let derived = if self
16187+
.failed_derived_table_factor_positions
16188+
.contains(&derived_pos)
1617716189
{
16190+
None
16191+
} else {
16192+
match self.maybe_parse(|parser| parser.parse_derived_table_factor(NotLateral))? {
16193+
Some(t) => Some(t),
16194+
None => {
16195+
self.failed_derived_table_factor_positions.insert(derived_pos);
16196+
None
16197+
}
16198+
}
16199+
};
16200+
if let Some(mut table) = derived {
1617816201
while let Some(kw) = self.parse_one_of_keywords(&[Keyword::PIVOT, Keyword::UNPIVOT])
1617916202
{
1618016203
table = match kw {

tests/sqlparser_common.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19111,3 +19111,34 @@ fn parse_prefix_case_chain_no_exponential_blowup() {
1911119111
rx.recv_timeout(Duration::from_secs(5))
1911219112
.expect("parser should reject this quickly, not loop exponentially");
1911319113
}
19114+
19115+
/// Regression test for the 2^N parse-time blowup in `parse_table_factor` on
19116+
/// inputs like `SELECT 1 FROM ((((...`. The speculative derived-table arm
19117+
/// and the nested-join fallback both recurse through the remaining paren
19118+
/// chain, doubling work per level. Post-fix the per-position failure cache
19119+
/// short-circuits the second descent.
19120+
#[test]
19121+
fn parse_table_factor_paren_chain_no_exponential_blowup() {
19122+
use std::sync::mpsc;
19123+
use std::thread;
19124+
use std::time::Duration;
19125+
19126+
let mut sql = String::from("SELECT 1 ");
19127+
for _ in 0..5 {
19128+
sql.push_str("FROM ");
19129+
sql.push_str(&"(".repeat(30));
19130+
sql.push(' ');
19131+
}
19132+
19133+
let (tx, rx) = mpsc::channel();
19134+
thread::spawn(move || {
19135+
let _ = Parser::new(&GenericDialect {})
19136+
.with_recursion_limit(256)
19137+
.try_with_sql(&sql)
19138+
.and_then(|mut p| p.parse_statements());
19139+
let _ = tx.send(());
19140+
});
19141+
19142+
rx.recv_timeout(Duration::from_secs(5))
19143+
.expect("parser should reject this quickly, not loop exponentially");
19144+
}

0 commit comments

Comments
 (0)