Skip to content

Commit 3a3b917

Browse files
authored
fix: Remove outdated depth warning (#22030)
1 parent 4dd8b47 commit 3a3b917

File tree

10 files changed

+27
-67
lines changed

10 files changed

+27
-67
lines changed

Diff for: Cargo.lock

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: crates/polars-expr/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ polars-time = { workspace = true, optional = true }
2424
polars-utils = { workspace = true }
2525
rand = { workspace = true }
2626
rayon = { workspace = true }
27+
recursive = { workspace = true }
2728

2829
[features]
2930
nightly = ["polars-core/nightly", "polars-plan/nightly"]

Diff for: crates/polars-expr/src/planner.rs

+6-38
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use polars_core::prelude::*;
22
use polars_plan::prelude::expr_ir::ExprIR;
33
use polars_plan::prelude::*;
4+
use recursive::recursive;
45

56
use crate::expressions as phys_expr;
67
use crate::expressions::*;
@@ -97,47 +98,28 @@ pub struct ExpressionConversionState {
9798
// settings per expression
9899
// those are reset every expression
99100
local: LocalConversionState,
100-
depth_limit: u16,
101101
}
102102

103-
#[derive(Copy, Clone)]
103+
#[derive(Copy, Clone, Default)]
104104
struct LocalConversionState {
105105
has_implode: bool,
106106
has_window: bool,
107107
has_lit: bool,
108-
// Max depth an expression may have.
109-
// 0 is unlimited.
110-
depth_limit: u16,
111-
}
112-
113-
impl Default for LocalConversionState {
114-
fn default() -> Self {
115-
Self {
116-
has_lit: false,
117-
has_implode: false,
118-
has_window: false,
119-
depth_limit: 500,
120-
}
121-
}
122108
}
123109

124110
impl ExpressionConversionState {
125-
pub fn new(allow_threading: bool, depth_limit: u16) -> Self {
111+
pub fn new(allow_threading: bool) -> Self {
126112
Self {
127-
depth_limit,
128113
allow_threading,
129114
has_windows: false,
130115
local: LocalConversionState {
131-
depth_limit,
132116
..Default::default()
133117
},
134118
}
135119
}
120+
136121
fn reset(&mut self) {
137-
self.local = LocalConversionState {
138-
depth_limit: self.depth_limit,
139-
..Default::default()
140-
}
122+
self.local = LocalConversionState::default();
141123
}
142124

143125
fn has_implode(&self) -> bool {
@@ -148,19 +130,6 @@ impl ExpressionConversionState {
148130
self.has_windows = true;
149131
self.local.has_window = true;
150132
}
151-
152-
fn check_depth(&mut self) {
153-
if self.local.depth_limit > 0 {
154-
self.local.depth_limit -= 1;
155-
156-
if self.local.depth_limit == 0 {
157-
let depth = get_expr_depth_limit().unwrap();
158-
polars_warn!(format!(
159-
"encountered expression deeper than {depth} elements; this may overflow the stack, consider refactoring"
160-
))
161-
}
162-
}
163-
}
164133
}
165134

166135
pub fn create_physical_expr(
@@ -183,6 +152,7 @@ pub fn create_physical_expr(
183152
}
184153
}
185154

155+
#[recursive]
186156
fn create_physical_expr_inner(
187157
expression: Node,
188158
ctxt: Context,
@@ -192,8 +162,6 @@ fn create_physical_expr_inner(
192162
) -> PolarsResult<Arc<dyn PhysicalExpr>> {
193163
use AExpr::*;
194164

195-
state.check_depth();
196-
197165
match expr_arena.get(expression) {
198166
Len => Ok(Arc::new(phys_expr::CountExpr::new())),
199167
Window {

Diff for: crates/polars-lazy/src/dsl/eval.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ pub trait ExprEvalExtension: IntoExpr + Sized {
6262
Context::Default,
6363
&arena,
6464
&schema,
65-
&mut ExpressionConversionState::new(true, 0),
65+
&mut ExpressionConversionState::new(true),
6666
)?;
6767

6868
let state = ExecutionState::new();

Diff for: crates/polars-lazy/src/frame/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -637,7 +637,7 @@ impl LazyFrame {
637637
Context::Default,
638638
expr_arena,
639639
schema,
640-
&mut ExpressionConversionState::new(true, 0),
640+
&mut ExpressionConversionState::new(true),
641641
)
642642
.ok()?;
643643
let io_expr = phys_expr_to_io_expr(phys_expr);

Diff for: crates/polars-lazy/src/physical_plan/exotic.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,6 @@ pub(crate) fn prepare_expression_for_context(
4343
ctxt,
4444
&expr_arena,
4545
&input_schema,
46-
&mut ExpressionConversionState::new(true, 0),
46+
&mut ExpressionConversionState::new(true),
4747
)
4848
}

Diff for: crates/polars-lazy/src/physical_plan/streaming/construct_pipeline.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ fn to_physical_piped_expr(
5656
Context::Default,
5757
expr_arena,
5858
schema,
59-
&mut ExpressionConversionState::new(false, 0),
59+
&mut ExpressionConversionState::new(false),
6060
)
6161
.map(|e| Arc::new(Wrap(e)) as Arc<dyn PhysicalPipedExpr>)
6262
}

Diff for: crates/polars-mem-engine/src/planner/lp.rs

+12-19
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,13 @@ fn partitionable_gb(
5353

5454
#[derive(Clone)]
5555
struct ConversionState {
56-
expr_depth: u16,
5756
has_cache_child: bool,
5857
has_cache_parent: bool,
5958
}
6059

6160
impl ConversionState {
6261
fn new() -> PolarsResult<Self> {
6362
Ok(ConversionState {
64-
expr_depth: get_expr_depth_limit()?,
6563
has_cache_child: false,
6664
has_cache_parent: false,
6765
})
@@ -212,7 +210,7 @@ fn create_physical_plan_impl(
212210
match logical_plan {
213211
#[cfg(feature = "python")]
214212
PythonScan { mut options } => {
215-
let mut expr_conv_state = ExpressionConversionState::new(true, state.expr_depth);
213+
let mut expr_conv_state = ExpressionConversionState::new(true);
216214
let (predicate, predicate_serialized) =
217215
python_scan_predicate(&mut options, expr_arena, &mut expr_conv_state)?;
218216
Ok(Box::new(executors::PythonScanExec {
@@ -457,7 +455,7 @@ fn create_physical_plan_impl(
457455
}
458456
}
459457
let input = recurse!(input, state)?;
460-
let mut state = ExpressionConversionState::new(true, state.expr_depth);
458+
let mut state = ExpressionConversionState::new(true);
461459
let predicate = create_physical_expr(
462460
&predicate,
463461
Context::Default,
@@ -488,7 +486,7 @@ fn create_physical_plan_impl(
488486
_set_n_rows_for_scan(None).map(|x| (0, x))
489487
};
490488

491-
let mut state = ExpressionConversionState::new(true, state.expr_depth);
489+
let mut state = ExpressionConversionState::new(true);
492490
let do_new_multifile = (sources.len() > 1 || hive_parts.is_some())
493491
&& !matches!(&*scan_type, FileScan::Anonymous { .. })
494492
&& std::env::var("POLARS_NEW_MULTIFILE").as_deref() == Ok("1");
@@ -601,10 +599,7 @@ fn create_physical_plan_impl(
601599
} => {
602600
let input_schema = lp_arena.get(input).schema(lp_arena).into_owned();
603601
let input = recurse!(input, state)?;
604-
let mut state = ExpressionConversionState::new(
605-
POOL.current_num_threads() > expr.len(),
606-
state.expr_depth,
607-
);
602+
let mut state = ExpressionConversionState::new(POOL.current_num_threads() > expr.len());
608603
let phys_expr = create_physical_expressions_from_irs(
609604
&expr,
610605
Context::Default,
@@ -648,7 +643,7 @@ fn create_physical_plan_impl(
648643
Context::Default,
649644
expr_arena,
650645
input_schema.as_ref(),
651-
&mut ExpressionConversionState::new(true, state.expr_depth),
646+
&mut ExpressionConversionState::new(true),
652647
)?;
653648
let input = recurse!(input, state)?;
654649
Ok(Box::new(executors::SortExec {
@@ -704,14 +699,14 @@ fn create_physical_plan_impl(
704699
Context::Default,
705700
expr_arena,
706701
&input_schema,
707-
&mut ExpressionConversionState::new(true, state.expr_depth),
702+
&mut ExpressionConversionState::new(true),
708703
)?;
709704
let phys_aggs = create_physical_expressions_from_irs(
710705
&aggs,
711706
Context::Aggregation,
712707
expr_arena,
713708
&input_schema,
714-
&mut ExpressionConversionState::new(true, state.expr_depth),
709+
&mut ExpressionConversionState::new(true),
715710
)?;
716711

717712
let _slice = options.slice;
@@ -820,14 +815,14 @@ fn create_physical_plan_impl(
820815
Context::Default,
821816
expr_arena,
822817
&schema_left,
823-
&mut ExpressionConversionState::new(true, state.expr_depth),
818+
&mut ExpressionConversionState::new(true),
824819
)?;
825820
let right_on = create_physical_expressions_from_irs(
826821
&right_on,
827822
Context::Default,
828823
expr_arena,
829824
&schema_right,
830-
&mut ExpressionConversionState::new(true, state.expr_depth),
825+
&mut ExpressionConversionState::new(true),
831826
)?;
832827
let options = Arc::try_unwrap(options).unwrap_or_else(|options| (*options).clone());
833828

@@ -842,7 +837,7 @@ fn create_physical_plan_impl(
842837
Context::Default,
843838
expr_arena,
844839
&schema,
845-
&mut ExpressionConversionState::new(false, state.expr_depth),
840+
&mut ExpressionConversionState::new(false),
846841
)?;
847842

848843
let execution_state = ExecutionState::default();
@@ -881,10 +876,8 @@ fn create_physical_plan_impl(
881876
.iter()
882877
.all(|e| is_elementwise_rec_no_cat_cast(expr_arena.get(e.node()), expr_arena));
883878

884-
let mut state = ExpressionConversionState::new(
885-
POOL.current_num_threads() > exprs.len(),
886-
state.expr_depth,
887-
);
879+
let mut state =
880+
ExpressionConversionState::new(POOL.current_num_threads() > exprs.len());
888881

889882
let phys_exprs = create_physical_expressions_from_irs(
890883
&exprs,

Diff for: crates/polars-stream/src/physical_plan/lower_expr.rs

+1-3
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ use polars_core::frame::DataFrame;
44
use polars_core::prelude::{DataType, Field, InitHashMaps, PlHashMap, PlHashSet};
55
use polars_core::schema::{Schema, SchemaExt};
66
use polars_error::PolarsResult;
7-
use polars_expr::planner::get_expr_depth_limit;
87
use polars_expr::state::ExecutionState;
98
use polars_expr::{ExpressionConversionState, create_physical_expr};
109
use polars_ops::frame::{JoinArgs, JoinType};
@@ -384,8 +383,7 @@ fn build_fallback_node_with_ctx(
384383
};
385384

386385
let output_schema = schema_for_select(input_stream, exprs, ctx)?;
387-
let expr_depth_limit = get_expr_depth_limit()?;
388-
let mut conv_state = ExpressionConversionState::new(false, expr_depth_limit);
386+
let mut conv_state = ExpressionConversionState::new(false);
389387
let phys_exprs = exprs
390388
.iter()
391389
.map(|expr| {

Diff for: crates/polars-stream/src/physical_plan/to_graph.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use polars_core::prelude::PlRandomState;
66
use polars_core::schema::Schema;
77
use polars_error::{PolarsResult, polars_bail, polars_ensure, polars_err};
88
use polars_expr::groups::new_hash_grouper;
9-
use polars_expr::planner::{ExpressionConversionState, create_physical_expr, get_expr_depth_limit};
9+
use polars_expr::planner::{ExpressionConversionState, create_physical_expr};
1010
use polars_expr::reduce::into_reduction;
1111
use polars_expr::state::ExecutionState;
1212
use polars_mem_engine::{create_physical_plan, create_scan_predicate};
@@ -75,13 +75,12 @@ pub fn physical_plan_to_graph(
7575
) -> PolarsResult<(Graph, SecondaryMap<PhysNodeKey, GraphNodeKey>)> {
7676
// Get the number of threads from the rayon thread-pool as that respects our config.
7777
let num_pipelines = POOL.current_num_threads();
78-
let expr_depth_limit = get_expr_depth_limit()?;
7978
let mut ctx = GraphConversionContext {
8079
phys_sm,
8180
expr_arena,
8281
graph: Graph::with_capacity(phys_sm.len()),
8382
phys_to_graph: SecondaryMap::with_capacity(phys_sm.len()),
84-
expr_conversion_state: ExpressionConversionState::new(false, expr_depth_limit),
83+
expr_conversion_state: ExpressionConversionState::new(false),
8584
num_pipelines,
8685
};
8786

0 commit comments

Comments
 (0)