Skip to content

Conversation

@sundy-li
Copy link
Member

@sundy-li sundy-li commented Jan 8, 2026

Summary

Replace `recursive` crate with `stacksafe` for preventing stack overflows in deeply recursive functions. StackSafe provides better stack management with additional safety features.

Changes

This PR replaces `recursive` crate with `stacksafe` crate (v1.0.0) throughout the codebase:

  1. Dependency updates:

    • Root `Cargo.toml`: `recursive = "0.1.1"` → `stacksafe = "1"`
    • Updated 6 sub-crate Cargo.toml files to use `stacksafe = { workspace = true }`
    • Removed git patch override for `recursive`
  2. Attribute migrations (300+ replacements across 137 files):

    • `#[recursive::recursive]` → `#[stacksafe::stacksafe]`
    • `#[async_recursion(#[recursive::recursive])]` → `#[async_recursion(#[stacksafe::stacksafe])]`
    • Educe derive attrs: `"#[recursive::recursive]"` → `"#[stacksafe::stacksafe]"`
  3. API call updates (in `flight_actions.rs`):

    • `recursive::get_minimum_stack_size()` → `stacksafe::get_minimum_stack_size()`
    • `recursive::get_stack_allocation_size()` → `stacksafe::get_stack_allocation_size()`
  4. New annotation:

    • Added `#[stacksafe::stacksafe]` to `try_rewrite_subquery` function to prevent potential stack overflow in deeply nested subqueries

Motivation

StackSafe offers several advantages over `recursive` crate:

  • Automatic trait safety: `StackSafe` wrapper provides stack-safe implementations for `Clone`, `Debug`, `Drop`, etc.
  • Debug-time checks: Verifies that `#[stacksafe]` is properly annotated on functions accessing recursive data structures
  • Better stack management: More reliable stack growth with configurable thresholds
  • Production proven: Used in production by Zed, ScopeDB, and other projects

Implementation

The migration is a straightforward attribute replacement with no functional changes:

  • StackSafe maintains API compatibility with `recursive` (same attribute names)
  • Default stack thresholds (128 KiB red zone, 2 MiB allocation) remain the same
  • All recursive functions and derived trait implementations are automatically protected

Tests

  • Unit Test - `cargo check` passes on all modified packages
  • Clippy - No clippy warnings
  • Format - `cargo fmt` passes
  • Build - `cargo check --package databend-query` compiles successfully

Type of change

  • Refactor (non-breaking change which improves code structure and safety)

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/


This change is Reviewable

@github-actions github-actions bot added the pr-refactor this PR changes the code base without new features or bugfix label Jan 8, 2026
@sundy-li sundy-li changed the title refactor: replace recursive crate with stacksafe for stack overflow prevention refactor(query): replace recursive crate with stacksafe for stack overflow prevention Jan 8, 2026
…revention

Replace the `recursive` crate with `stacksafe` crate for preventing
stack overflows in deeply recursive functions. StackSafe provides
better stack management with additional features:

- Automatic trait safety for recursive data structures
- Debug-time safety checks to catch missing annotations
- Same API surface for easy migration

Changes:
- Update Cargo.toml: recursive 0.1.1 -> stacksafe 1.0.0
- Replace all #[recursive::recursive] with #[stacksafe::stacksafe]
- Replace all educe derive attrs using recursive::recursive
- Update API calls (get_minimum_stack_size, get_stack_allocation_size)
- Add #[stacksafe::stacksafe] to try_rewrite_subquery function to
  prevent potential stack overflow in deeply nested subqueries

This migration affects 137 files with 300+ attribute replacements.
@sundy-li sundy-li force-pushed the feat/replace-recursive-with-stacksafe branch 3 times, most recently from e412b49 to 91c0aab Compare January 9, 2026 01:03
@github-actions
Copy link
Contributor

github-actions bot commented Jan 9, 2026

🤖 CI Job Analysis

Workflow: 20837738473

📊 Summary

  • Total Jobs: 23
  • Failed Jobs: 3
  • Retryable: 0
  • Code Issues: 3

NO RETRY NEEDED

All failures appear to be code/test issues requiring manual fixes.

🔍 Job Details

  • linux / test_unit: Not retryable (Code/Test)
  • linux / check: Not retryable (Code/Test)
  • linux / build: Not retryable (Code/Test)

🤖 About

Automated analysis using job annotations to distinguish infrastructure issues (auto-retried) from code/test issues (manual fixes needed).

@sundy-li sundy-li force-pushed the feat/replace-recursive-with-stacksafe branch 4 times, most recently from efb3f4c to 666b74b Compare January 9, 2026 01:10
@sundy-li sundy-li force-pushed the feat/replace-recursive-with-stacksafe branch from 666b74b to 5626060 Compare January 9, 2026 01:13
@sundy-li sundy-li marked this pull request as draft January 9, 2026 04:06
@sundy-li sundy-li closed this Jan 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-refactor this PR changes the code base without new features or bugfix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant