This is an automated email from the ASF dual-hosted git repository.
alamb pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git
The following commit(s) were added to refs/heads/master by this push:
new bf48122c8 Minimize stack space required to plan deeply nested binary
expressions (#4787)
bf48122c8 is described below
commit bf48122c83bd038624ccfe708ac03ea4688ef170
Author: Andrew Lamb <[email protected]>
AuthorDate: Mon Jan 2 10:05:21 2023 -0500
Minimize stack space required to plan deeply nested binary expressions
(#4787)
* Update datafusion/core/tests/tpcds_planning.rs
* Fix last tpds overflow issues
* lint
* Apply suggestions from code review
* Update datafusion/core/tests/tpcds_planning.rs
---
datafusion/sql/src/planner.rs | 42 ++++++++++++++++++++++++++++++++++++++----
1 file changed, 38 insertions(+), 4 deletions(-)
diff --git a/datafusion/sql/src/planner.rs b/datafusion/sql/src/planner.rs
index ee116a7b8..7b06790a8 100644
--- a/datafusion/sql/src/planner.rs
+++ b/datafusion/sql/src/planner.rs
@@ -1839,6 +1839,37 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
sql: SQLExpr,
schema: &DFSchema,
planner_context: &mut PlannerContext,
+ ) -> Result<Expr> {
+ // Workaround for
https://github.com/apache/arrow-datafusion/issues/4065
+ //
+ // Minimize stack space required in debug builds to plan
+ // deeply nested binary operators by keeping the stack space
+ // needed for sql_expr_to_logical_expr minimal for BinaryOp
+ //
+ // The reason this reduces stack size in debug builds is
+ // explained in the "Technical Backstory" heading of
+ // https://github.com/apache/arrow-datafusion/pull/1047
+ //
+ // A likely better way to support deeply nested expressions
+ // would be to avoid recursion all together and use an
+ // iterative algorithm.
+ match sql {
+ SQLExpr::BinaryOp { left, op, right } => {
+ self.parse_sql_binary_op(*left, op, *right, schema,
planner_context)
+ }
+ // since this function requires more space per frame
+ // avoid calling it for binary ops
+ _ => self.sql_expr_to_logical_expr_internal(sql, schema,
planner_context),
+ }
+ }
+
+ /// Internal implementation. Use
+ /// [`Self::sql_expr_to_logical_expr`] to plan exprs.
+ fn sql_expr_to_logical_expr_internal(
+ &self,
+ sql: SQLExpr,
+ schema: &DFSchema,
+ planner_context: &mut PlannerContext,
) -> Result<Expr> {
match sql {
SQLExpr::Value(value) => {
@@ -1976,10 +2007,13 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
SQLExpr::BinaryOp {
- left,
- op,
- right,
- } => self.parse_sql_binary_op(*left, op, *right, schema,
planner_context),
+ ..
+ } => {
+ Err(DataFusionError::Internal(
+ "binary_op should be handled by
sql_expr_to_logical_expr.".to_string()
+ ))
+ }
+
#[cfg(feature = "unicode_expressions")]
SQLExpr::Substring {
