Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add #[recursive] #1522

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
4 changes: 3 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,16 @@ path = "src/lib.rs"

[features]
default = ["std"]
std = []
std = ["recursive"]
# Enable JSON output in the `cli` example:
json_example = ["serde_json", "serde"]
visitor = ["sqlparser_derive"]

[dependencies]
bigdecimal = { version = "0.4.1", features = ["serde"], optional = true }
log = "0.4"
recursive = { version = "0.1.1", optional = true}

serde = { version = "1.0", features = ["derive"], optional = true }
# serde_json is only used in examples/cli, but we have to put it outside
# of dev-dependencies because of
Expand Down
1 change: 1 addition & 0 deletions derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ fn derive_visit(input: proc_macro::TokenStream, visit_type: &VisitType) -> proc_
let expanded = quote! {
// The generated impl.
impl #impl_generics sqlparser::ast::#visit_trait for #name #ty_generics #where_clause {
#[cfg_attr(feature = "std", recursive::recursive)]
fn visit<V: sqlparser::ast::#visitor_trait>(
&#modifier self,
visitor: &mut V
Expand Down
40 changes: 40 additions & 0 deletions sqlparser_bench/benches/sqlparser_bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,46 @@ fn basic_queries(c: &mut Criterion) {
group.bench_function("sqlparser::with_select", |b| {
b.iter(|| Parser::parse_sql(&dialect, with_query));
});

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For large_statement, making separated test would make differentiating potential(future) regression easier.
It's a suggestion(fine to leave it as it is).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understood your comment, sorry. I added tests for parsing large statements in tests/sqlparser_common.rs. Do you think we should test something else?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thinks it would be better to add as separated test.

let large_statement = {
let expressions = (0..1000)
.map(|n| format!("FN_{}(COL_{})", n, n))
.collect::<Vec<_>>()
.join(", ");
let tables = (0..1000)
.map(|n| format!("TABLE_{}", n))
.collect::<Vec<_>>()
.join(" JOIN ");
let where_condition = (0..1000)
.map(|n| format!("COL_{} = {}", n, n))
.collect::<Vec<_>>()
.join(" OR ");
let order_condition = (0..1000)
.map(|n| format!("COL_{} DESC", n))
.collect::<Vec<_>>()
.join(", ");

format!(
"SELECT {} FROM {} WHERE {} ORDER BY {}",
expressions, tables, where_condition, order_condition
)
};

group.bench_function("parse_large_statement", |b| {
b.iter(|| Parser::parse_sql(&dialect, criterion::black_box(large_statement.as_str())));
});

let large_statement = Parser::parse_sql(&dialect, large_statement.as_str())
.unwrap()
.pop()
.unwrap();

group.bench_function("format_large_statement", |b| {
b.iter(|| {
let formatted_query = large_statement.to_string();
assert_eq!(formatted_query, large_statement);
});
});
}

criterion_group!(benches, basic_queries);
Expand Down
1 change: 1 addition & 0 deletions src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1188,6 +1188,7 @@ impl fmt::Display for CastFormat {
}

impl fmt::Display for Expr {
#[cfg_attr(feature = "std", recursive::recursive)]
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
Expr::Identifier(s) => write!(f, "{s}"),
Expand Down
25 changes: 25 additions & 0 deletions src/ast/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -884,4 +884,29 @@ mod tests {
assert_eq!(actual, expected)
}
}

struct QuickVisitor; // [`TestVisitor`] is too slow to iterate over thousands of nodes

impl Visitor for QuickVisitor {
type Break = ();
}

#[test]
fn overflow() {
let cond = (0..1000)
blaginin marked this conversation as resolved.
Show resolved Hide resolved
.map(|n| format!("X = {}", n))
.collect::<Vec<_>>()
.join(" OR ");
let sql = format!("SELECT x where {0}", cond);

let dialect = GenericDialect {};
let tokens = Tokenizer::new(&dialect, sql.as_str()).tokenize().unwrap();
let s = Parser::new(&dialect)
.with_tokens(tokens)
.parse_statement()
.unwrap();

let mut visitor = QuickVisitor {};
s.visit(&mut visitor);
}
}
13 changes: 13 additions & 0 deletions tests/sqlparser_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11748,3 +11748,16 @@ fn parse_create_table_select() {
);
}
}

#[test]
fn overflow() {
let expr = std::iter::repeat("1")
.take(1000)
blaginin marked this conversation as resolved.
Show resolved Hide resolved
.collect::<Vec<_>>()
.join(" + ");
let sql = format!("SELECT {}", expr);

let mut statements = Parser::parse_sql(&GenericDialect {}, sql.as_str()).unwrap();
let statement = statements.pop().unwrap();
assert_eq!(statement.to_string(), sql);
}
Loading