Merge pull request #13776 from Mytherin/expressiondepth

Make binder expression depth limit use `max_expression_depth`

Author: Mytherin

src/include/duckdb/planner/expression_binder.hpp

@@ -154,8 +154,6 @@ class ExpressionBinder {
 	static LogicalType GetExpressionReturnType(const Expression &expr);
 
 private:
-	//! Maximum stack depth
-	static constexpr const idx_t MAXIMUM_STACK_DEPTH = 128;
 	//! Current stack depth
 	idx_t stack_depth = DConstants::INVALID_INDEX;
 
@@ -204,6 +202,8 @@ class ExpressionBinder {
 	virtual BindResult BindUnnest(FunctionExpression &expr, idx_t depth, bool root_expression);
 	virtual BindResult BindMacro(FunctionExpression &expr, ScalarMacroCatalogEntry &macro, idx_t depth,
 	                             unique_ptr<ParsedExpression> &expr_ptr);
+	void UnfoldMacroExpression(FunctionExpression &function, ScalarMacroCatalogEntry &macro_func,
+	                           unique_ptr<ParsedExpression> &expr);
 
 	virtual string UnsupportedAggregateMessage();
 	virtual string UnsupportedUnnestMessage();

src/planner/binder/expression/bind_macro_expression.cpp

@@ -99,8 +99,8 @@ void ExpressionBinder::ReplaceMacroParameters(unique_ptr<ParsedExpression> &expr
 	    *expr, [&](unique_ptr<ParsedExpression> &child) { ReplaceMacroParameters(child, lambda_params); });
 }
 
-BindResult ExpressionBinder::BindMacro(FunctionExpression &function, ScalarMacroCatalogEntry &macro_func, idx_t depth,
-                                       unique_ptr<ParsedExpression> &expr) {
+void ExpressionBinder::UnfoldMacroExpression(FunctionExpression &function, ScalarMacroCatalogEntry &macro_func,
+                                             unique_ptr<ParsedExpression> &expr) {
 	// validate the arguments and separate positional and default arguments
 	vector<unique_ptr<ParsedExpression>> positionals;
 	unordered_map<string, unique_ptr<ParsedExpression>> defaults;
@@ -143,6 +143,14 @@ BindResult ExpressionBinder::BindMacro(FunctionExpression &function, ScalarMacro
 	// now replace the parameters
 	vector<unordered_set<string>> lambda_params;
 	ReplaceMacroParameters(expr, lambda_params);
+}
+
+BindResult ExpressionBinder::BindMacro(FunctionExpression &function, ScalarMacroCatalogEntry &macro_func, idx_t depth,
+                                       unique_ptr<ParsedExpression> &expr) {
+	auto stack_checker = StackCheck(*expr, 3);
+
+	// unfold the macro expression
+	UnfoldMacroExpression(function, macro_func, expr);
 
 	// bind the unfolded macro
 	return BindExpression(expr, depth);

src/planner/expression_binder.cpp

@@ -7,6 +7,7 @@
 #include "duckdb/planner/expression/list.hpp"
 #include "duckdb/planner/expression_iterator.hpp"
 #include "duckdb/common/operator/cast_operators.hpp"
+#include "duckdb/main/client_config.hpp"
 
 namespace duckdb {
 
@@ -36,18 +37,21 @@ ExpressionBinder::~ExpressionBinder() {
 }
 
 void ExpressionBinder::InitializeStackCheck() {
+	static constexpr idx_t INITIAL_DEPTH = 5;
 	if (binder.HasActiveBinder()) {
-		stack_depth = binder.GetActiveBinder().stack_depth;
+		stack_depth = binder.GetActiveBinder().stack_depth + INITIAL_DEPTH;
 	} else {
-		stack_depth = 0;
+		stack_depth = INITIAL_DEPTH;
 	}
 }
 
 StackChecker<ExpressionBinder> ExpressionBinder::StackCheck(const ParsedExpression &expr, idx_t extra_stack) {
 	D_ASSERT(stack_depth != DConstants::INVALID_INDEX);
-	if (stack_depth + extra_stack >= MAXIMUM_STACK_DEPTH) {
-		throw BinderException("Maximum recursion depth exceeded (Maximum: %llu) while binding \"%s\"",
-		                      MAXIMUM_STACK_DEPTH, expr.ToString());
+	auto &options = ClientConfig::GetConfig(context);
+	if (stack_depth + extra_stack >= options.max_expression_depth) {
+		throw BinderException("Max expression depth limit of %lld exceeded. Use \"SET max_expression_depth TO x\" to "
+		                      "increase the maximum expression depth.",
+		                      options.max_expression_depth);
 	}
 	return StackChecker<ExpressionBinder>(*this, extra_stack);
 }

test/sql/catalog/function/test_recursive_macro.test

@@ -11,12 +11,12 @@ CREATE MACRO "sum"(x) AS (CASE WHEN sum(x) IS NULL THEN 0 ELSE sum(x) END);
 statement error
 SELECT sum(1);
 ----
-Binder Error: Maximum recursion depth exceeded
+Max expression depth limit
 
 statement error
 SELECT sum(1) WHERE 42=0
 ----
-Binder Error: Maximum recursion depth exceeded
+Max expression depth limit
 
 statement ok
 DROP MACRO sum

test/sql/catalog/function/test_recursive_macro_no_dependency.test

@@ -8,12 +8,12 @@ CREATE MACRO "sum"(x) AS (CASE WHEN sum(x) IS NULL THEN 0 ELSE sum(x) END);
 statement error
 SELECT sum(1);
 ----
-Binder Error: Maximum recursion depth exceeded
+Max expression depth limit
 
 statement error
 SELECT sum(1) WHERE 42=0
 ----
-Binder Error: Maximum recursion depth exceeded
+Max expression depth limit
 
 statement ok
 DROP MACRO sum

test/sql/parser/expression_depth_limit.test

@@ -6,11 +6,12 @@ statement ok
 SELECT (1+(1+(1+(1+(1+(1+(1+1)))))));
 
 statement ok
-SET max_expression_depth TO 3;
+SET max_expression_depth TO 7;
 
 statement error
 SELECT (1+(1+(1+(1+(1+(1+(1+1)))))));
 ----
+expression depth limit
 
 statement ok
 SET max_expression_depth TO 1000;