mdashti commented on code in PR #23198:
URL: https://github.com/apache/datafusion/pull/23198#discussion_r3483636782
##########
datafusion/sql/src/expr/mod.rs:
##########
@@ -1539,30 +1539,42 @@ mod tests {
)))
}
+ fn or_chain_of(num_terms: usize) -> SQLExpr {
+ let expr_str = (0..num_terms)
+ .map(|i| format!("column1 = 'value{i}'"))
+ .collect::<Vec<String>>()
+ .join(" OR ");
+ Parser::new(&GenericDialect {})
+ .try_with_sql(&expr_str)
+ .unwrap()
+ .parse_expr()
+ .unwrap()
+ }
+
macro_rules! test_stack_overflow {
($name:ident, $num_expr:expr) => {
+ // Planning a long binary chain stays iterative (see #1444), but
the resulting `Expr` is
+ // as deep as the chain and drops recursively, so run on a large
stack to give that drop
+ // room.
#[test]
fn $name() {
- let schema = DFSchema::empty();
- let mut planner_context = PlannerContext::default();
-
- let expr_str = (0..$num_expr)
- .map(|i| format!("column1 = 'value{:?}'", i))
- .collect::<Vec<String>>()
- .join(" OR ");
-
- let dialect = GenericDialect {};
- let mut parser = Parser::new(&dialect)
- .try_with_sql(expr_str.as_str())
- .unwrap();
- let sql_expr = parser.parse_expr().unwrap();
-
- let context_provider = TestContextProvider::new();
- let sql_to_rel = SqlToRel::new(&context_provider);
-
- // Should not stack overflow
- sql_to_rel
- .sql_expr_to_logical_expr(sql_expr, &schema, &mut
planner_context)
+ std::thread::Builder::new()
+ .stack_size(64 * 1024 * 1024)
Review Comment:
Thanks. You're right. It's a real production issue, so I went with the
iterative `Drop`. The `BinaryExpr` children hold a `BoxedExpr` whose `Drop`
tears a deep chain down off the call stack (`39e458c5c`), so the original
`test_stack_overflow` cases pass with no stack trick. `impl Drop for Expr`
wasn't viable (`E0509` from the by-value destructuring in `map_children`), so
the newtype wraps just the recursive edges. Repurposed the PR for it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
