adriangb commented on code in PR #23198:
URL: https://github.com/apache/datafusion/pull/23198#discussion_r3482732818


##########
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:
   wdyt of this reply: "doesn't this defeat the purpose of the test? can we 
keep the smaller stack size for the planning (which is what we are trying to 
ensure is iterative) but move the value into a large stack thread to run 
`drop()`?"



-- 
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]

Reply via email to