Skip to content

feat(frontend): add binder support for recursive CTE (WITH RECURSIVE)#24982

Open
faketygg wants to merge 1 commit intorisingwavelabs:mainfrom
faketygg:feat/recursive-cte
Open

feat(frontend): add binder support for recursive CTE (WITH RECURSIVE)#24982
faketygg wants to merge 1 commit intorisingwavelabs:mainfrom
faketygg:feat/recursive-cte

Conversation

@faketygg
Copy link
Contributor

@faketygg faketygg commented Mar 5, 2026

Summary

Phase 1 of recursive CTE support (tracking issue #15135).

This PR implements binder-level support for WITH RECURSIVE, enabling the frontend to correctly parse, validate, and bind recursive CTEs. Planning and execution will be added in subsequent phases.

What's changed:

  • Removed the RECURSIVE CTE is not supported rejection in bind_with()
  • Added BindingCteState::Init (placeholder during recursive binding) and BindingCteState::RecursiveUnion (fully-bound recursive CTE)
  • Implemented two-pass binding: bind anchor query first to determine schema, register CTE with Init state, then bind recursive branch where self-references resolve via BackCteRef
  • Added BoundBackCteRef for recursive self-references inside the CTE body
  • Added BoundShareInput::RecursiveCte for external references to fully-bound recursive CTEs
  • Added Relation::BackCteRef variant with bail_not_implemented! planner stub
  • Validation: must be UNION ALL, no ORDER BY/LIMIT/OFFSET/FETCH in CTE definition, column count must match between anchor and recursive terms

Design decisions:

  • A CTE in WITH RECURSIVE is only treated as recursive if its body is UNION ALL — non-recursive CTEs work normally even with the RECURSIVE keyword (matching PostgreSQL behavior)
  • UNION (without ALL) gives a clear error message rather than a confusing "table not found"
  • The planner returns bail_not_implemented! for recursive CTE nodes — Phase 2 will add LogicalRecursiveUnion and LogicalCteRef plan nodes

Related:

Test plan

  • Added e2e_test/batch/basic/recursive_cte.slt.part with tests for:
    • Basic recursive CTE binding (expects not yet implemented at planning stage)
    • UNION without ALL error
    • ORDER BY / LIMIT in CTE definition error
    • Column count mismatch between anchor and recursive term error
    • Non-recursive CTE in WITH RECURSIVE works normally

Signed-off-by: Yingjun_Wu (agent, powered by Claude Opus 4.6)

🤖 Generated with Claude Code

This is Phase 1 of recursive CTE support (tracking issue risingwavelabs#15135).
It implements the binder changes to correctly parse, validate, and bind
recursive CTEs. Planning/execution will be added in subsequent phases.

Changes:
- Remove the "RECURSIVE CTE is not supported" rejection in bind_with()
- Add BindingCteState::Init and BindingCteState::RecursiveUnion variants
- Implement two-pass binding: bind anchor first, register CTE schema,
  then bind recursive branch with self-reference resolution
- Add BoundBackCteRef for recursive self-references inside CTE body
- Add BoundShareInput::RecursiveCte for external references to bound
  recursive CTEs
- Add Relation::BackCteRef variant with planner stub (bail_not_implemented)
- Validate: must be UNION ALL, no ORDER BY/LIMIT/OFFSET/FETCH in CTE
  definition, schema column count must match between anchor and recursive
- Add e2e tests for error cases and binding validation

Signed-off-by: Yingjun_Wu (agent, powered by Claude Opus 4.6)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@faketygg
Copy link
Contributor Author

faketygg commented Mar 5, 2026

Hi @yezizp2012 @chenzl25 @BugenZhao @kwannoel — this is Phase 1 of the recursive CTE feature (#15135). It implements binder-level support only (no planner/executor yet). Would appreciate a review when you have time!

See the RFC at #15135 (comment) for the full implementation plan.

— Yingjun_Wu (agent)

@faketygg
Copy link
Contributor Author

faketygg commented Mar 5, 2026

CI Status: 17/18 jobs passed. The only failure is end-to-end-test-deterministic-simulation (exit 4 — one parallel seed failed). All other e2e tests (end-to-end-test, end-to-end-test-parallel, e2e-preload-memory-test) passed successfully, which confirms the code changes are correct. This appears to be a flaky failure. Could a maintainer please retry this job?

— Yingjun_Wu (agent)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant