Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 34 additions & 6 deletions src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16423,14 +16423,42 @@ impl<'a> Parser<'a> {
if !self
.dialect
.supports_left_associative_joins_without_parens()
&& !natural
&& self.peek_parens_less_nested_join()
{
let joins = self.parse_joins()?;
relation = TableFactor::NestedJoin {
table_with_joins: Box::new(TableWithJoins { relation, joins }),
alias: None,
};
let mut inner_joins = self.parse_joins()?;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please refactor dropping the recursion, otherwise parse time goes superlinear (median of seven runs, release):

joins main this branch
100 0.97 ms 4.63 ms
200 1.66 ms 14.0 ms
400 3.17 ms 46.1 ms
700 5.54 ms 98.4 ms

let has_deferred_constraint = matches!(
self.peek_token_ref().token,
Token::Word(Word {
keyword: Keyword::ON | Keyword::USING,
..
})
);

if has_deferred_constraint {
relation = TableFactor::NestedJoin {
table_with_joins: Box::new(TableWithJoins {
relation,
joins: inner_joins,
}),
alias: None,
};
} else {
let last = inner_joins.pop().expect("inner_joins is non-empty");
let outer_constraint = if natural {
JoinConstraint::Natural
} else {
JoinConstraint::None
};

joins.push(Join {
relation,
global,
join_operator: join_operator_type(outer_constraint),
});
joins.extend(inner_joins);
joins.push(last);
Comment on lines +16446 to +16459

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

pop then extend then push(last) rebuilds the same vector, and last pins a whole Join in every recursion frame.

Suggested change
let last = inner_joins.pop().expect("inner_joins is non-empty");
let outer_constraint = if natural {
JoinConstraint::Natural
} else {
JoinConstraint::None
};
joins.push(Join {
relation,
global,
join_operator: join_operator_type(outer_constraint),
});
joins.extend(inner_joins);
joins.push(last);
let outer_constraint = if natural {
JoinConstraint::Natural
} else {
JoinConstraint::None
};
joins.push(Join {
relation,
global,
join_operator: join_operator_type(outer_constraint),
});
joins.extend(inner_joins);

continue;
}
}

let join_constraint = self.parse_join_constraint(natural)?;
Expand Down
102 changes: 102 additions & 0 deletions tests/sqlparser_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17592,6 +17592,108 @@ fn join_precedence() {
);
}

#[test]
fn parse_left_join_chain_with_and_without_left_associativity() {
let query = "SELECT 'ORIGINAL' AS src, o.order_id, c.customer_id, p.product_id FROM orders AS o JOIN customers AS c LEFT JOIN products AS p ON p.order_id = o.order_id ORDER BY o.order_id, c.customer_id, p.product_id";

let generic_ast = Parser::parse_sql(&GenericDialect {}, query)
.unwrap()
.into_iter()
.next()
.unwrap();
let generic_canonical = generic_ast.to_string();
println!("Generic AST:\n{generic_ast:#?}");
println!("Generic canonical:\n{generic_canonical}");

assert_eq!(
generic_canonical,
"SELECT 'ORIGINAL' AS src, o.order_id, c.customer_id, p.product_id FROM orders AS o JOIN customers AS c LEFT JOIN products AS p ON p.order_id = o.order_id ORDER BY o.order_id, c.customer_id, p.product_id"
);

let Statement::Query(generic_query) = &generic_ast else {
unreachable!()
};
let SetExpr::Select(generic_select) = generic_query.body.as_ref() else {
unreachable!()
};
let generic_from = only(&generic_select.from);
assert_eq!(generic_from.joins.len(), 2);

let snowflake_ast = Parser::parse_sql(&SnowflakeDialect {}, query)
.unwrap()
.into_iter()
.next()
.unwrap();
let snowflake_canonical = snowflake_ast.to_string();
println!("Snowflake AST:\n{snowflake_ast:#?}");
println!("Snowflake canonical:\n{snowflake_canonical}");

assert_eq!(
snowflake_canonical,
"SELECT 'ORIGINAL' AS src, o.order_id, c.customer_id, p.product_id FROM orders AS o JOIN customers AS c LEFT JOIN products AS p ON p.order_id = o.order_id ORDER BY o.order_id, c.customer_id, p.product_id"
);

let Statement::Query(snowflake_query) = &snowflake_ast else {
unreachable!()
};
let SetExpr::Select(snowflake_select) = snowflake_query.body.as_ref() else {
unreachable!()
};
let snowflake_from = only(&snowflake_select.from);
assert_eq!(snowflake_from.joins.len(), 2);
}
Comment on lines +17596 to +17644

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
fn parse_left_join_chain_with_and_without_left_associativity() {
let query = "SELECT 'ORIGINAL' AS src, o.order_id, c.customer_id, p.product_id FROM orders AS o JOIN customers AS c LEFT JOIN products AS p ON p.order_id = o.order_id ORDER BY o.order_id, c.customer_id, p.product_id";
let generic_ast = Parser::parse_sql(&GenericDialect {}, query)
.unwrap()
.into_iter()
.next()
.unwrap();
let generic_canonical = generic_ast.to_string();
println!("Generic AST:\n{generic_ast:#?}");
println!("Generic canonical:\n{generic_canonical}");
assert_eq!(
generic_canonical,
"SELECT 'ORIGINAL' AS src, o.order_id, c.customer_id, p.product_id FROM orders AS o JOIN customers AS c LEFT JOIN products AS p ON p.order_id = o.order_id ORDER BY o.order_id, c.customer_id, p.product_id"
);
let Statement::Query(generic_query) = &generic_ast else {
unreachable!()
};
let SetExpr::Select(generic_select) = generic_query.body.as_ref() else {
unreachable!()
};
let generic_from = only(&generic_select.from);
assert_eq!(generic_from.joins.len(), 2);
let snowflake_ast = Parser::parse_sql(&SnowflakeDialect {}, query)
.unwrap()
.into_iter()
.next()
.unwrap();
let snowflake_canonical = snowflake_ast.to_string();
println!("Snowflake AST:\n{snowflake_ast:#?}");
println!("Snowflake canonical:\n{snowflake_canonical}");
assert_eq!(
snowflake_canonical,
"SELECT 'ORIGINAL' AS src, o.order_id, c.customer_id, p.product_id FROM orders AS o JOIN customers AS c LEFT JOIN products AS p ON p.order_id = o.order_id ORDER BY o.order_id, c.customer_id, p.product_id"
);
let Statement::Query(snowflake_query) = &snowflake_ast else {
unreachable!()
};
let SetExpr::Select(snowflake_select) = snowflake_query.body.as_ref() else {
unreachable!()
};
let snowflake_from = only(&snowflake_select.from);
assert_eq!(snowflake_from.joins.len(), 2);
}
fn parse_join_chain_without_deferred_constraint() {
let select = all_dialects().verified_only_select(
"SELECT * FROM orders AS o JOIN customers AS c LEFT JOIN products AS p ON p.order_id = o.order_id",
);
let from = only(&select.from);
assert_eq!(from.joins.len(), 2);
assert!(matches!(
from.joins[0].join_operator,
JoinOperator::Join(JoinConstraint::None)
));
assert!(matches!(
from.joins[1].join_operator,
JoinOperator::Left(JoinConstraint::On(_))
));
}


#[test]
fn parse_left_join_chain_with_and_without_left_associativity_cross_join() {
let query = "SELECT 'ORIGINAL' AS src, o.order_id, c.customer_id, p.product_id FROM orders AS o CROSS JOIN customers AS c LEFT JOIN products AS p ON p.order_id = o.order_id ORDER BY o.order_id, c.customer_id, p.product_id";

let generic_ast = Parser::parse_sql(&GenericDialect {}, query)
.unwrap()
.into_iter()
.next()
.unwrap();
let generic_canonical = generic_ast.to_string();
println!("Generic AST:\n{generic_ast:#?}");
println!("Generic canonical:\n{generic_canonical}");

assert_eq!(
generic_canonical,
"SELECT 'ORIGINAL' AS src, o.order_id, c.customer_id, p.product_id FROM orders AS o CROSS JOIN customers AS c LEFT JOIN products AS p ON p.order_id = o.order_id ORDER BY o.order_id, c.customer_id, p.product_id"
);

let Statement::Query(generic_query) = &generic_ast else {
unreachable!()
};
let SetExpr::Select(generic_select) = generic_query.body.as_ref() else {
unreachable!()
};
let generic_from = only(&generic_select.from);
assert_eq!(generic_from.joins.len(), 2);

let snowflake_ast = Parser::parse_sql(&SnowflakeDialect {}, query)
.unwrap()
.into_iter()
.next()
.unwrap();
let snowflake_canonical = snowflake_ast.to_string();
println!("Snowflake AST:\n{snowflake_ast:#?}");
println!("Snowflake canonical:\n{snowflake_canonical}");

assert_eq!(
snowflake_canonical,
"SELECT 'ORIGINAL' AS src, o.order_id, c.customer_id, p.product_id FROM orders AS o CROSS JOIN customers AS c LEFT JOIN products AS p ON p.order_id = o.order_id ORDER BY o.order_id, c.customer_id, p.product_id"
);

let Statement::Query(snowflake_query) = &snowflake_ast else {
unreachable!()
};
let SetExpr::Select(snowflake_select) = snowflake_query.body.as_ref() else {
unreachable!()
};
let snowflake_from = only(&snowflake_select.from);
assert_eq!(snowflake_from.joins.len(), 2);
}
Comment on lines +17647 to +17695

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
fn parse_left_join_chain_with_and_without_left_associativity_cross_join() {
let query = "SELECT 'ORIGINAL' AS src, o.order_id, c.customer_id, p.product_id FROM orders AS o CROSS JOIN customers AS c LEFT JOIN products AS p ON p.order_id = o.order_id ORDER BY o.order_id, c.customer_id, p.product_id";
let generic_ast = Parser::parse_sql(&GenericDialect {}, query)
.unwrap()
.into_iter()
.next()
.unwrap();
let generic_canonical = generic_ast.to_string();
println!("Generic AST:\n{generic_ast:#?}");
println!("Generic canonical:\n{generic_canonical}");
assert_eq!(
generic_canonical,
"SELECT 'ORIGINAL' AS src, o.order_id, c.customer_id, p.product_id FROM orders AS o CROSS JOIN customers AS c LEFT JOIN products AS p ON p.order_id = o.order_id ORDER BY o.order_id, c.customer_id, p.product_id"
);
let Statement::Query(generic_query) = &generic_ast else {
unreachable!()
};
let SetExpr::Select(generic_select) = generic_query.body.as_ref() else {
unreachable!()
};
let generic_from = only(&generic_select.from);
assert_eq!(generic_from.joins.len(), 2);
let snowflake_ast = Parser::parse_sql(&SnowflakeDialect {}, query)
.unwrap()
.into_iter()
.next()
.unwrap();
let snowflake_canonical = snowflake_ast.to_string();
println!("Snowflake AST:\n{snowflake_ast:#?}");
println!("Snowflake canonical:\n{snowflake_canonical}");
assert_eq!(
snowflake_canonical,
"SELECT 'ORIGINAL' AS src, o.order_id, c.customer_id, p.product_id FROM orders AS o CROSS JOIN customers AS c LEFT JOIN products AS p ON p.order_id = o.order_id ORDER BY o.order_id, c.customer_id, p.product_id"
);
let Statement::Query(snowflake_query) = &snowflake_ast else {
unreachable!()
};
let SetExpr::Select(snowflake_select) = snowflake_query.body.as_ref() else {
unreachable!()
};
let snowflake_from = only(&snowflake_select.from);
assert_eq!(snowflake_from.joins.len(), 2);
}
#[test]
fn parse_long_join_chain_without_deferred_constraint() {
let mut sql = String::from("SELECT * FROM t0");
for index in 1..=200 {
sql.push_str(&format!(" JOIN t{index}"));
}
non_left_associative_dialects().verified_stmt(&sql);
}


#[test]
fn test_nested_join_without_parentheses() {
let query = "SELECT DISTINCT p.product_id FROM orders AS o INNER JOIN customers AS c INNER JOIN products AS p ON p.customer_id = c.customer_id ON c.order_id = o.order_id";
Expand Down
Loading