Skip to content

Conversation

@Tpt
Copy link
Contributor

@Tpt Tpt commented Nov 26, 2025

It gives a name (the table name) to each WorkTable. This way WorkTableExec can recognize its own WorkTable

Note that it doesn't allow multiple occurrences of the same CTE name: it's not possible to implement things like "join with itself" correctly with only the work table.

Which issue does this PR close?

Rationale for this change

Support nested recursive CTEs without co-recursion. This is useful to e.g. implement SPARQL or other graph query languages.

What changes are included in this PR?

Are these changes tested?

Yes! There is a nested recursive query in the test file

Are there any user-facing changes?

Nested recursive queries are now allowed instead of failing with a "not implemented" error

@github-actions github-actions bot added sqllogictest SQL Logic Tests (.slt) physical-plan Changes to the physical-plan crate labels Nov 26, 2025
It gives a name (the table name) to each WorkTable. This way WorkTableExec can recognize its own WorkTable

Note that it doesn't allow multiple occurrences of the same CTE name: it's not possible to implement things like "join with itself" correctly with only the work table.
@Tpt Tpt force-pushed the tpt/nested-recursive-ctes branch from 6170e0e to b2587ee Compare November 26, 2025 21:04
@Tpt Tpt marked this pull request as ready for review November 27, 2025 07:48
Copy link
Member

@martin-g martin-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍🏻

One question: Should name shadowing in nested CTEs be supported ?
Some databases (e.g. Postgres) support it, others - don't.
If it is supported then it would be good to add a test case.
If it is not then it would be good to document it somewhere.

@Tpt
Copy link
Contributor Author

Tpt commented Nov 27, 2025

@martin-g Thank you!

One question: Should name shadowing in nested CTEs be supported ?

This is a good point. Thank you! It's not supported yet, might be nice to support it. I have added a test in 0db8728 Currently it raises "Error during planning: WITH query name "outer_cte" specified more than once" which sounds like a fair error message.
CTEs are currently not documented, I have added a comment to #16935 to think to cover this case.

@Jefffrey Jefffrey added this pull request to the merge queue Dec 25, 2025
Merged via the queue into apache:main with commit ea2e22c Dec 25, 2025
31 checks passed
@Jefffrey
Copy link
Contributor

Thanks @Tpt & @martin-g

@Tpt Tpt deleted the tpt/nested-recursive-ctes branch December 25, 2025 20:09
@Tpt
Copy link
Contributor Author

Tpt commented Dec 26, 2025

@Jefffrey Thank you so much for the review! There is also #18993 that fixes projection in the same operator.

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

Labels

physical-plan Changes to the physical-plan crate sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support nested recursive CTEs

3 participants