stack overflow in physical executor with very deep expression trees

[wence-]

Checks

• I have checked that this issue has not already been reported.
• I have confirmed this bug exists on the latest version of Polars.

Reproducible example

use polars::prelude::*;
use polars::lazy::dsl::col;

fn main() -> Result<(), PolarsError> {
    let df = df!("a" => &[1])?.lazy();
    let expr = (0..3200).fold(col("a"), |acc, _x| acc + col("a"));
    let out = df.select([expr]);
    println!("{}", &out.collect()?);
    Ok(())
}

Log output

$ POLARS_VERBOSE=1 ./target/release/polars-doodles 

thread 'polars-0' has overflowed its stack
fatal runtime error: stack overflow

Aborted (core dumped)

Issue description

Observed when trying to get another datapoint for #16224. Stack overflow in deep recursion when evaluating the physical plan:

$ gdb --args ./target/release/polars-doodles
(gdb) run
...
Thread 2 "polars-0" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffff7c74640 (LWP 556609)]
0x00005555558246a7 in rayon_core::join::join_context::{{closure}} ()
(gdb) bt
#0  0x00005555558246a7 in rayon_core::join::join_context::{{closure}} ()
#1  0x000055555594659b in <polars_lazy::physical_plan::expressions::binary::BinaryExpr as polars_lazy::physical_plan::expressions::PhysicalExpr>::evaluate ()
#2  0x0000555555824820 in rayon_core::join::join_context::{{closure}} ()
#3  0x000055555594659b in <polars_lazy::physical_plan::expressions::binary::BinaryExpr as polars_lazy::physical_plan::expressions::PhysicalExpr>::evaluate ()
#4  0x0000555555824820 in rayon_core::join::join_context::{{closure}} ()
#5  0x000055555594659b in <polars_lazy::physical_plan::expressions::binary::BinaryExpr as polars_lazy::physical_plan::expressions::PhysicalExpr>::evaluate ()
#6  0x0000555555824820 in rayon_core::join::join_context::{{closure}} ()
#7  0x000055555594659b in <polars_lazy::physical_plan::expressions::binary::BinaryExpr as polars_lazy::physical_plan::expressions::PhysicalExpr>::evaluate ()
#8  0x0000555555824820 in rayon_core::join::join_context::{{closure}} ()
#9  0x000055555594659b in <polars_lazy::physical_plan::expressions::binary::BinaryExpr as polars_lazy::physical_plan::expressions::PhysicalExpr>::evaluate ()
#10 0x0000555555824820 in rayon_core::join::join_context::{{closure}} ()
#11 0x000055555594659b in <polars_lazy::physical_plan::expressions::binary::BinaryExpr as polars_lazy::physical_plan::expressions::PhysicalExpr>::evaluate ()
#12 0x0000555555824820 in rayon_core::join::join_context::{{closure}} ()
#13 0x000055555594659b in <polars_lazy::physical_plan::expressions::binary::BinaryExpr as polars_lazy::physical_plan::expressions::PhysicalExpr>::evaluate ()
#14 0x0000555555824820 in rayon_core::join::join_context::{{closure}} ()
#15 0x000055555594659b in <polars_lazy::physical_plan::expressions::binary::BinaryExpr as polars_lazy::physical_plan::expressions::PhysicalExpr>::evaluate ()
#16 0x0000555555824820 in rayon_core::join::join_context::{{closure}} ()
#17 0x000055555594659b in <polars_lazy::physical_plan::expressions::binary::BinaryExpr as polars_lazy::physical_plan::expressions::PhysicalExpr>::evaluate ()
...
#6373 0x000055555594659b in <polars_lazy::physical_plan::expressions::binary::BinaryExpr as polars_lazy::physical_plan::expressions::PhysicalExpr>::evaluate ()
#6374 0x0000555555824820 in rayon_core::join::join_context::{{closure}} ()
#6375 0x00005555558cd629 in <rayon_core::job::StackJob<L,F,R> as rayon_core::job::Job>::execute ()
#6376 0x000055555574292f in rayon_core::registry::WorkerThread::wait_until_cold ()
#6377 0x00005555567e0752 in rayon_core::registry::ThreadBuilder::run ()
#6378 0x00005555567e3a5a in std::sys_common::backtrace::__rust_begin_short_backtrace ()
#6379 0x00005555567e497f in core::ops::function::FnOnce::call_once{{vtable.shim}} ()
#6380 0x00005555568e9cf5 in std::sys::pal::unix::thread::Thread::new::thread_start ()

Expected behavior

I suppose I would be happy with a WONTFIX here, though perhaps there could be a warning at some point in the planner when observing very deep expression trees that this is likely to be a problem and the user should refactor their expressions.

Installed versions

[dependencies]
polars = { version = "0.39.2", features = ["lazy"] }

[ritchie46]

Yeah, I agree having a tunable soft-limit might be a good idea. I do think it should remain a warning though, as finishing the query and potentially succeeding seems better to me than raising. During the conversion to physical plan of the default engine we can catch this. Those are recursive.

[wence-]

Thanks!