From f822df8acd17630b74b6899df1521702c53714eb Mon Sep 17 00:00:00 2001 From: Dandandan Date: Sun, 28 Jun 2020 21:32:26 +0200 Subject: [PATCH 1/5] Add SQL that takes very long to benchmark --- sqlparser_bench/benches/sqlparser_bench.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/sqlparser_bench/benches/sqlparser_bench.rs b/sqlparser_bench/benches/sqlparser_bench.rs index 5293c0f50..687bb16a9 100644 --- a/sqlparser_bench/benches/sqlparser_bench.rs +++ b/sqlparser_bench/benches/sqlparser_bench.rs @@ -37,6 +37,19 @@ fn basic_queries(c: &mut Criterion) { group.bench_function("sqlparser::with_select", |b| { b.iter(|| Parser::parse_sql(&dialect, with_query)); }); + + let nested_query = " + SELECT +@ FROM((((((((((SELECT +@ FROM((((((SELECT +@ FROM((((((((((SELECT +@ FROM(((((((((((((SELECT +I FROM(((((((SELECT +I FROM +"; +group.bench_function("sqlparser::nested_query", |b| { + b.iter(|| Parser::parse_sql(&dialect, nested_query)); +}); } criterion_group!(benches, basic_queries); From a198f8d5b174bb0a2ff611a5a63057504c47792b Mon Sep 17 00:00:00 2001 From: Dandandan Date: Tue, 30 Jun 2020 09:12:27 +0200 Subject: [PATCH 2/5] Memoize unsuccesful usage of `parse_derived_table_factor` at a certain index --- src/parser.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index a1e542484..018720394 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -21,7 +21,7 @@ use super::dialect::Dialect; use super::tokenizer::*; use std::error::Error; use std::fmt; - +use std::collections::HashSet; #[derive(Debug, Clone, PartialEq)] pub enum ParserError { TokenizerError(String), @@ -87,12 +87,14 @@ pub struct Parser { tokens: Vec, /// The index of the first unprocessed token in `self.tokens` index: usize, + /// Memoizes unsuccesful `parse_derived_table_factor` results + memoize_parse_derived_table_factor: HashSet, } impl Parser { /// Parse the specified tokens pub fn new(tokens: Vec) -> Self { - Parser { tokens, index: 0 } + Parser { tokens, index: 0, memoize_parse_derived_table_factor: HashSet::new() } } /// Parse a SQL statement and produce an Abstract Syntax Tree (AST) @@ -2054,10 +2056,14 @@ impl Parser { // If the recently consumed '(' starts a derived table, the call to // `parse_derived_table_factor` below will return success after parsing the // subquery, followed by the closing ')', and the alias of the derived table. - // In the example above this is case (3). - return_ok_if_some!( - self.maybe_parse(|parser| parser.parse_derived_table_factor(NotLateral)) - ); + // In the example above this is case (3).const + if !self.memoize_parse_derived_table_factor.contains(&self.index) { + return_ok_if_some!( + self.maybe_parse(|parser| parser.parse_derived_table_factor(NotLateral)) + ); + } + self.memoize_parse_derived_table_factor.insert(self.index); + // A parsing error from `parse_derived_table_factor` indicates that the '(' we've // recently consumed does not start a derived table (cases 1, 2, or 4). // `maybe_parse` will ignore such an error and rewind to be after the opening '('. From 2eb3f8d1bd1a56f66c6aa85a36f5224758736182 Mon Sep 17 00:00:00 2001 From: Dandandan Date: Tue, 30 Jun 2020 09:13:40 +0200 Subject: [PATCH 3/5] Formatting --- src/parser.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index 018720394..760d7232d 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -19,9 +19,9 @@ use super::dialect::keywords; use super::dialect::keywords::Keyword; use super::dialect::Dialect; use super::tokenizer::*; +use std::collections::HashSet; use std::error::Error; use std::fmt; -use std::collections::HashSet; #[derive(Debug, Clone, PartialEq)] pub enum ParserError { TokenizerError(String), @@ -94,7 +94,11 @@ pub struct Parser { impl Parser { /// Parse the specified tokens pub fn new(tokens: Vec) -> Self { - Parser { tokens, index: 0, memoize_parse_derived_table_factor: HashSet::new() } + Parser { + tokens, + index: 0, + memoize_parse_derived_table_factor: HashSet::new(), + } } /// Parse a SQL statement and produce an Abstract Syntax Tree (AST) @@ -2057,7 +2061,10 @@ impl Parser { // `parse_derived_table_factor` below will return success after parsing the // subquery, followed by the closing ')', and the alias of the derived table. // In the example above this is case (3).const - if !self.memoize_parse_derived_table_factor.contains(&self.index) { + if !self + .memoize_parse_derived_table_factor + .contains(&self.index) + { return_ok_if_some!( self.maybe_parse(|parser| parser.parse_derived_table_factor(NotLateral)) ); From 599c4f2d3051fd10e4fe9783d94bb278bee27013 Mon Sep 17 00:00:00 2001 From: Dandandan Date: Tue, 30 Jun 2020 09:17:45 +0200 Subject: [PATCH 4/5] Add a extra comment --- src/parser.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/parser.rs b/src/parser.rs index 760d7232d..df03abf32 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -2060,7 +2060,10 @@ impl Parser { // If the recently consumed '(' starts a derived table, the call to // `parse_derived_table_factor` below will return success after parsing the // subquery, followed by the closing ')', and the alias of the derived table. - // In the example above this is case (3).const + // In the example above this is case (3). + + // We will only try to parse `parse_derived_table_factor` if it wasn't tried in a + // previous attempt for the same index if !self .memoize_parse_derived_table_factor .contains(&self.index) From 68fb30cfa208b4f9c25c89c242d0a877bdb16f0a Mon Sep 17 00:00:00 2001 From: Dandandan Date: Tue, 30 Jun 2020 09:22:59 +0200 Subject: [PATCH 5/5] Move insert --- sqlparser_bench/benches/sqlparser_bench.rs | 6 +++--- src/parser.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sqlparser_bench/benches/sqlparser_bench.rs b/sqlparser_bench/benches/sqlparser_bench.rs index 687bb16a9..8237ee550 100644 --- a/sqlparser_bench/benches/sqlparser_bench.rs +++ b/sqlparser_bench/benches/sqlparser_bench.rs @@ -47,9 +47,9 @@ fn basic_queries(c: &mut Criterion) { I FROM(((((((SELECT I FROM "; -group.bench_function("sqlparser::nested_query", |b| { - b.iter(|| Parser::parse_sql(&dialect, nested_query)); -}); + group.bench_function("sqlparser::nested_query", |b| { + b.iter(|| Parser::parse_sql(&dialect, nested_query)); + }); } criterion_group!(benches, basic_queries); diff --git a/src/parser.rs b/src/parser.rs index df03abf32..edfb06061 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -2071,8 +2071,8 @@ impl Parser { return_ok_if_some!( self.maybe_parse(|parser| parser.parse_derived_table_factor(NotLateral)) ); + self.memoize_parse_derived_table_factor.insert(self.index); } - self.memoize_parse_derived_table_factor.insert(self.index); // A parsing error from `parse_derived_table_factor` indicates that the '(' we've // recently consumed does not start a derived table (cases 1, 2, or 4).