Skip to content

Commit 77e0832

Browse files
BohuTANGUbuntuclaude
authored
fix(query): prevent stack overflow in PhysicalPlan recursion (#18589)
* fix(query): prevent stack overflow in PhysicalPlan recursion Addresses the stack overflow issue introduced in commit c0f12f9 where the migration from enum-based to trait-based PhysicalPlan architecture caused deep recursion problems. Key changes: 1. Remove dangerous #[recursive::recursive] annotations from Clone::clone() and other methods that could cause infinite recursion with dyn_clone 2. Replace recursive methods with depth-limited versions: - formatter() -> formatter_with_depth() - try_find_mutation_source() -> try_find_mutation_source_with_depth() - get_all_data_source() -> get_all_data_source_with_depth() - set_pruning_stats() -> set_pruning_stats_with_depth() - is_distributed_plan() -> is_distributed_plan_with_depth() - is_warehouse_distributed_plan() -> is_warehouse_distributed_plan_with_depth() 3. Add MAX_DEPTH=1000 safety limit with graceful degradation 4. Maintain backward compatibility through public API wrappers This prevents the SIGSEGV crashes seen in CI tests while preserving the trait-based architecture benefits. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * fix(query): increase MAX_DEPTH to 4096 for consistency with binder Increases the depth limit from 1000 to 4096 to align with the existing query binder depth limit used elsewhere in the codebase. This provides better support for complex query plans while maintaining safety. - Consistent with bind_context.rs MAX_DEPTH = 4096 - Better handling of complex nested queries - Maintains the same safety guarantees * fix(query): clarify impact of depth-limited set_pruning_stats Clarifies that when depth limit is exceeded in set_pruning_stats_with_depth(), the early return prevents stack overflow while preserving correctness. Note: This may affect query performance optimization in extremely deep query plans (>4096 levels) as some pruning statistics may not be applied to the deepest nodes, but query results remain correct. The trade-off prioritizes system stability over optimization in edge cases. * fix(query): remove recursive annotation from PhysicalPlan Clone Remove #[recursive::recursive] annotation from Clone implementation to prevent stack overflow when cloning complex physical plans. The combination of dyn_clone trait objects and recursive annotation was causing infinite recursion leading to SIGSEGV crashes in CI tests. This is a minimal, targeted fix focusing on the root cause identified from the crash stack traces showing 50+ repeated Clone calls. Other recursive annotations are preserved as they have proper termination conditions and don't cause infinite recursion. --------- Co-authored-by: Ubuntu <[email protected]> Co-authored-by: Claude <[email protected]>
1 parent 7cca5f5 commit 77e0832

File tree

1 file changed

+0
-1
lines changed

1 file changed

+0
-1
lines changed

src/query/service/src/physical_plans/physical_plan.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,6 @@ pub struct PhysicalPlan {
276276
dyn_clone::clone_trait_object!(IPhysicalPlan);
277277

278278
impl Clone for PhysicalPlan {
279-
#[recursive::recursive]
280279
fn clone(&self) -> Self {
281280
PhysicalPlan {
282281
inner: self.inner.clone(),

0 commit comments

Comments
 (0)