Skip to content

Commit e6f5cb6

Browse files
buraksennalamb
buraksenn
and
alamb
authored
[minor] make recursive package dependency optional (#13778)
* make recursive optional * add to default for common package * cargo update * added to readme * make test conditional * reviews * cargo update --------- Co-authored-by: Andrew Lamb <[email protected]>
1 parent a267784 commit e6f5cb6

File tree

17 files changed

+54
-50
lines changed

17 files changed

+54
-50
lines changed

‎README.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,8 @@ Default features:
112112
- `parquet`: support for reading the [Apache Parquet] format
113113
- `regex_expressions`: regular expression functions, such as `regexp_match`
114114
- `unicode_expressions`: Include unicode aware functions such as `character_length`
115-
- `unparser` : enables support to reverse LogicalPlans back into SQL
115+
- `unparser`: enables support to reverse LogicalPlans back into SQL
116+
- `recursive-protection`: uses [recursive](https://docs.rs/recursive/latest/recursive/) for stack overflow protection.
116117

117118
Optional features:
118119

‎datafusion-cli/Cargo.lock‎

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎datafusion/common/Cargo.toml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,12 @@ name = "datafusion_common"
3636
path = "src/lib.rs"
3737

3838
[features]
39+
default = ["recursive-protection"]
3940
avro = ["apache-avro"]
4041
backtrace = []
4142
pyarrow = ["pyo3", "arrow/pyarrow", "parquet"]
4243
force_hash_collisions = []
44+
recursive-protection = ["dep:recursive"]
4345

4446
[dependencies]
4547
ahash = { workspace = true }
@@ -62,7 +64,7 @@ object_store = { workspace = true, optional = true }
6264
parquet = { workspace = true, optional = true, default-features = true }
6365
paste = "1.0.15"
6466
pyo3 = { version = "0.22.0", optional = true }
65-
recursive = { workspace = true }
67+
recursive = { workspace = true, optional = true }
6668
sqlparser = { workspace = true }
6769
tokio = { workspace = true }
6870

‎datafusion/common/src/tree_node.rs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
//! [`TreeNode`] for visiting and rewriting expression and plan trees
1919
2020
use crate::Result;
21-
use recursive::recursive;
2221
use std::collections::HashMap;
2322
use std::hash::Hash;
2423
use std::sync::Arc;
@@ -125,7 +124,7 @@ pub trait TreeNode: Sized {
125124
/// TreeNodeVisitor::f_up(ChildNode2)
126125
/// TreeNodeVisitor::f_up(ParentNode)
127126
/// ```
128-
#[recursive]
127+
#[cfg_attr(feature = "recursive-protection", recursive::recursive)]
129128
fn visit<'n, V: TreeNodeVisitor<'n, Node = Self>>(
130129
&'n self,
131130
visitor: &mut V,
@@ -175,7 +174,7 @@ pub trait TreeNode: Sized {
175174
/// TreeNodeRewriter::f_up(ChildNode2)
176175
/// TreeNodeRewriter::f_up(ParentNode)
177176
/// ```
178-
#[recursive]
177+
#[cfg_attr(feature = "recursive-protection", recursive::recursive)]
179178
fn rewrite<R: TreeNodeRewriter<Node = Self>>(
180179
self,
181180
rewriter: &mut R,
@@ -198,7 +197,7 @@ pub trait TreeNode: Sized {
198197
&'n self,
199198
mut f: F,
200199
) -> Result<TreeNodeRecursion> {
201-
#[recursive]
200+
#[cfg_attr(feature = "recursive-protection", recursive::recursive)]
202201
fn apply_impl<'n, N: TreeNode, F: FnMut(&'n N) -> Result<TreeNodeRecursion>>(
203202
node: &'n N,
204203
f: &mut F,
@@ -233,7 +232,7 @@ pub trait TreeNode: Sized {
233232
self,
234233
mut f: F,
235234
) -> Result<Transformed<Self>> {
236-
#[recursive]
235+
#[cfg_attr(feature = "recursive-protection", recursive::recursive)]
237236
fn transform_down_impl<N: TreeNode, F: FnMut(N) -> Result<Transformed<N>>>(
238237
node: N,
239238
f: &mut F,
@@ -257,7 +256,7 @@ pub trait TreeNode: Sized {
257256
self,
258257
mut f: F,
259258
) -> Result<Transformed<Self>> {
260-
#[recursive]
259+
#[cfg_attr(feature = "recursive-protection", recursive::recursive)]
261260
fn transform_up_impl<N: TreeNode, F: FnMut(N) -> Result<Transformed<N>>>(
262261
node: N,
263262
f: &mut F,
@@ -372,7 +371,7 @@ pub trait TreeNode: Sized {
372371
mut f_down: FD,
373372
mut f_up: FU,
374373
) -> Result<Transformed<Self>> {
375-
#[recursive]
374+
#[cfg_attr(feature = "recursive-protection", recursive::recursive)]
376375
fn transform_down_up_impl<
377376
N: TreeNode,
378377
FD: FnMut(N) -> Result<Transformed<N>>,
@@ -2350,6 +2349,7 @@ pub(crate) mod tests {
23502349
Ok(())
23512350
}
23522351

2352+
#[cfg(feature = "recursive-protection")]
23532353
#[test]
23542354
fn test_large_tree() {
23552355
let mut item = TestTreeNode::new_leaf("initial".to_string());

‎datafusion/expr/Cargo.toml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ name = "datafusion_expr"
3636
path = "src/lib.rs"
3737

3838
[features]
39+
default = ["recursive-protection"]
40+
recursive-protection = ["dep:recursive"]
3941

4042
[dependencies]
4143
arrow = { workspace = true }
@@ -48,7 +50,7 @@ datafusion-functions-window-common = { workspace = true }
4850
datafusion-physical-expr-common = { workspace = true }
4951
indexmap = { workspace = true }
5052
paste = "^1.0"
51-
recursive = { workspace = true }
53+
recursive = { workspace = true, optional = true }
5254
serde_json = { workspace = true }
5355
sqlparser = { workspace = true }
5456

‎datafusion/expr/src/expr_schema.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ use datafusion_common::{
3232
TableReference,
3333
};
3434
use datafusion_functions_window_common::field::WindowUDFFieldArgs;
35-
use recursive::recursive;
3635
use std::collections::HashMap;
3736
use std::sync::Arc;
3837

@@ -100,7 +99,7 @@ impl ExprSchemable for Expr {
10099
/// expression refers to a column that does not exist in the
101100
/// schema, or when the expression is incorrectly typed
102101
/// (e.g. `[utf8] + [bool]`).
103-
#[recursive]
102+
#[cfg_attr(feature = "recursive-protection", recursive::recursive)]
104103
fn get_type(&self, schema: &dyn ExprSchema) -> Result<DataType> {
105104
match self {
106105
Expr::Alias(Alias { expr, name, .. }) => match &**expr {

‎datafusion/expr/src/logical_plan/tree_node.rs‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ use crate::{
4545
UserDefinedLogicalNode, Values, Window,
4646
};
4747
use datafusion_common::tree_node::TreeNodeRefContainer;
48-
use recursive::recursive;
4948

5049
use crate::expr::{Exists, InSubquery};
5150
use datafusion_common::tree_node::{
@@ -669,7 +668,7 @@ impl LogicalPlan {
669668

670669
/// Visits a plan similarly to [`Self::visit`], including subqueries that
671670
/// may appear in expressions such as `IN (SELECT ...)`.
672-
#[recursive]
671+
#[cfg_attr(feature = "recursive-protection", recursive::recursive)]
673672
pub fn visit_with_subqueries<V: for<'n> TreeNodeVisitor<'n, Node = Self>>(
674673
&self,
675674
visitor: &mut V,
@@ -688,7 +687,7 @@ impl LogicalPlan {
688687
/// Similarly to [`Self::rewrite`], rewrites this node and its inputs using `f`,
689688
/// including subqueries that may appear in expressions such as `IN (SELECT
690689
/// ...)`.
691-
#[recursive]
690+
#[cfg_attr(feature = "recursive-protection", recursive::recursive)]
692691
pub fn rewrite_with_subqueries<R: TreeNodeRewriter<Node = Self>>(
693692
self,
694693
rewriter: &mut R,
@@ -707,7 +706,7 @@ impl LogicalPlan {
707706
&self,
708707
mut f: F,
709708
) -> Result<TreeNodeRecursion> {
710-
#[recursive]
709+
#[cfg_attr(feature = "recursive-protection", recursive::recursive)]
711710
fn apply_with_subqueries_impl<
712711
F: FnMut(&LogicalPlan) -> Result<TreeNodeRecursion>,
713712
>(
@@ -742,7 +741,7 @@ impl LogicalPlan {
742741
self,
743742
mut f: F,
744743
) -> Result<Transformed<Self>> {
745-
#[recursive]
744+
#[cfg_attr(feature = "recursive-protection", recursive::recursive)]
746745
fn transform_down_with_subqueries_impl<
747746
F: FnMut(LogicalPlan) -> Result<Transformed<LogicalPlan>>,
748747
>(
@@ -767,7 +766,7 @@ impl LogicalPlan {
767766
self,
768767
mut f: F,
769768
) -> Result<Transformed<Self>> {
770-
#[recursive]
769+
#[cfg_attr(feature = "recursive-protection", recursive::recursive)]
771770
fn transform_up_with_subqueries_impl<
772771
F: FnMut(LogicalPlan) -> Result<Transformed<LogicalPlan>>,
773772
>(
@@ -795,7 +794,7 @@ impl LogicalPlan {
795794
mut f_down: FD,
796795
mut f_up: FU,
797796
) -> Result<Transformed<Self>> {
798-
#[recursive]
797+
#[cfg_attr(feature = "recursive-protection", recursive::recursive)]
799798
fn transform_down_up_with_subqueries_impl<
800799
FD: FnMut(LogicalPlan) -> Result<Transformed<LogicalPlan>>,
801800
FU: FnMut(LogicalPlan) -> Result<Transformed<LogicalPlan>>,

‎datafusion/optimizer/Cargo.toml‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ workspace = true
3535
name = "datafusion_optimizer"
3636
path = "src/lib.rs"
3737

38+
[features]
39+
default = ["recursive-protection"]
40+
recursive-protection = ["dep:recursive"]
41+
3842
[dependencies]
3943
arrow = { workspace = true }
4044
chrono = { workspace = true }
@@ -44,7 +48,7 @@ datafusion-physical-expr = { workspace = true }
4448
indexmap = { workspace = true }
4549
itertools = { workspace = true }
4650
log = { workspace = true }
47-
recursive = { workspace = true }
51+
recursive = { workspace = true, optional = true }
4852
regex = { workspace = true }
4953
regex-syntax = "0.8.0"
5054

‎datafusion/optimizer/src/analyzer/subquery.rs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
use crate::analyzer::check_plan;
1919
use crate::utils::collect_subquery_cols;
20-
use recursive::recursive;
2120

2221
use datafusion_common::tree_node::{TreeNode, TreeNodeRecursion};
2322
use datafusion_common::{plan_err, Result};
@@ -79,7 +78,7 @@ pub fn check_subquery_expr(
7978
match outer_plan {
8079
LogicalPlan::Projection(_)
8180
| LogicalPlan::Filter(_) => Ok(()),
82-
LogicalPlan::Aggregate(Aggregate {group_expr, aggr_expr,..}) => {
81+
LogicalPlan::Aggregate(Aggregate { group_expr, aggr_expr, .. }) => {
8382
if group_expr.contains(expr) && !aggr_expr.contains(expr) {
8483
// TODO revisit this validation logic
8584
plan_err!(
@@ -88,7 +87,7 @@ pub fn check_subquery_expr(
8887
} else {
8988
Ok(())
9089
}
91-
},
90+
}
9291
_ => plan_err!(
9392
"Correlated scalar subquery can only be used in Projection, Filter, Aggregate plan nodes"
9493
)
@@ -129,7 +128,7 @@ fn check_correlations_in_subquery(inner_plan: &LogicalPlan) -> Result<()> {
129128
}
130129

131130
// Recursively check the unsupported outer references in the sub query plan.
132-
#[recursive]
131+
#[cfg_attr(feature = "recursive-protection", recursive::recursive)]
133132
fn check_inner_plan(inner_plan: &LogicalPlan, can_contain_outer_ref: bool) -> Result<()> {
134133
if !can_contain_outer_ref && inner_plan.contains_outer_reference() {
135134
return plan_err!("Accessing outer reference columns is not allowed in the plan");

‎datafusion/optimizer/src/common_subexpr_eliminate.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ use std::fmt::Debug;
2222
use std::sync::Arc;
2323

2424
use crate::{OptimizerConfig, OptimizerRule};
25-
use recursive::recursive;
2625

2726
use crate::optimizer::ApplyOrder;
2827
use crate::utils::NamePreserver;
@@ -532,7 +531,7 @@ impl OptimizerRule for CommonSubexprEliminate {
532531
None
533532
}
534533

535-
#[recursive]
534+
#[cfg_attr(feature = "recursive-protection", recursive::recursive)]
536535
fn rewrite(
537536
&self,
538537
plan: LogicalPlan,
@@ -952,7 +951,7 @@ mod test {
952951
)?
953952
.build()?;
954953

955-
let expected ="Aggregate: groupBy=[[]], aggr=[[avg(__common_expr_1) AS col1, my_agg(__common_expr_1) AS col2]]\
954+
let expected = "Aggregate: groupBy=[[]], aggr=[[avg(__common_expr_1) AS col1, my_agg(__common_expr_1) AS col2]]\
956955
\n Projection: UInt32(1) + test.a AS __common_expr_1, test.a, test.b, test.c\
957956
\n TableScan: test";
958957

0 commit comments

Comments
 (0)