Add support for recursive CTEs in ActiveRecord #51601
Conversation
ClearlyClaire
force-pushed
the
features/with-recursive
branch
from
d1e18c1
to
0289251
Compare
| # TODO: actually test recursive behavior | ||
| relation = Post | ||
| .with.recursive(posts_with_comments: Post.where("legacy_comments_count > 0")) | ||
| .from("posts_with_comments AS posts") | ||
|
|
||
| assert_equal POSTS_WITH_COMMENTS, relation.order(:id).pluck(:id) |
There was a problem hiding this comment.
I'm not familiar with the test fixtures and could not find ones that lend themselves to testing actual recursive CTEs.
ClearlyClaire
force-pushed
the
features/with-recursive
branch
from
0289251
to
073cb48
Compare
|
FYI: @vlado |
| class WithChain | ||
| def initialize(scope) # :nodoc: | ||
| @scope = scope | ||
| end | ||
|
|
||
| # Returns a new relation in which Common Table Expressions (CTEs) are flagged as recursive. | ||
| # | ||
| # See QueryMethods#with for more details. | ||
| def recursive(*args) | ||
| @scope.with_values += args | ||
| @scope.with_is_recursive = true | ||
| @scope | ||
| end | ||
| end |
There was a problem hiding this comment.
Not sure we need this. I think .with_recursive is just as fine as .with.recursive.
Unless I'm missing something?
There was a problem hiding this comment.
I was thinking .with.recursive was more in line with existing query methods, but from a usage perspective, with_recursive would work as well and should make the implementation simpler. I will switch to that in another commit I can then squash if we decide this is the way to go.
There was a problem hiding this comment.
Yeah, I assume you were thinking of where.not / where.associated / where.missing.
But in that case, I doubt we'll ever need multiple with_, but I see how as a enduser it might appear inconsistent :/
There was a problem hiding this comment.
I also think .with.recursive is more in line with existing query methods and would prefer that one.
Having said that, as @ClearlyClaire pointed out .with_recursive is simpler to implement (due to missing union support in ActiveRecord) and I prefer passing relations as positional arguments.
Post.with.recursive(
post_and_replies: [
Post.where(id: 42),
Post.joins('JOIN post_and_replies ON posts.in_reply_to_id = post_and_replies.id'),
]
)
# vs
Post.with_recursive(:post_and_replies, Post.where(id: 42), Post.joins('JOIN post_and_replies ON posts.in_reply_to_id = post_and_replies.id')
Or maybe we should add union support before implementing this 😉
There was a problem hiding this comment.
I have pushed a commit to change to QueryMethods#with_recursive, which works exactly the same way as QueryMethods#with except it also sets an instance variable to mark the WITH with RECURSIVE.
It's a separate commit so we can easily go back to with.recursive if that is deemed better.
Having said that, as @ClearlyClaire pointed out
.with_recursiveis simpler to implement (due to missing union support in ActiveRecord) and I prefer passing relations as positional arguments.
I actually prefer the array because it seems to me that it is more consistent with QueryMethods#with, and also allows passing a single SQL snippet that does the UNION, should the user prefer doing that. That being said, even with positional arguments we could make the last one optional to allow that.
There was a problem hiding this comment.
🤔 with_recursive seems better here, since chaining would serve only one purpose.
ClearlyClaire
force-pushed
the
features/with-recursive
branch
from
95e3310
to
cab51fc
Compare
byroot
force-pushed
the
features/with-recursive
branch
from
cab51fc
to
f47aba7
Compare
|
I added the missing test as well as a CHANGELOG entry. I'll merge on green. |
```ruby
Post.with_recursive(
post_and_replies: [
Post.where(id: 42),
Post.joins('JOIN post_and_replies ON posts.in_reply_to_id = post_and_replies.id'),
]
)
```
Generates the following SQL:
```sql
WITH RECURSIVE "post_and_replies" AS (
(SELECT "posts".* FROM "posts" WHERE "posts"."id" = 42)
UNION ALL
(SELECT "posts".* FROM "posts" JOIN post_and_replies ON posts.in_reply_to_id = post_and_replies.id)
)
SELECT "posts".* FROM "posts"
```
byroot
force-pushed
the
features/with-recursive
branch
from
f47aba7
to
fc26e44
Compare
Motivation / Background
While Rails 7.1 has added support for writing Common Table Expressions, this support does not extend to recursive CTEs.
This PR adds a
QueryMethods#with_recursiveconstruct to enable recursive CTEs.In addition, it enables passing arrays of queries as
withhash values, as recursive CTEs are generally only useful withUNIONorUNION ALL, which, to my knowledge, are currently not exposed outside of Arel.Detail
This Pull Request adds a
WithChainplaceholder object to chainwithandrecursive, in the manner ofwhere.not. CallingQueryMethods#with_recursivereturns a scope with awith_is_recursiveflag turned on, which is then used inbuild_withto pass:recursiveto Arel.The Pull Request also changes
build_with_value_from_hashto allow arrays of values as queries, in which case the subqueries are joined withUNION ALL.This enables writing queries such as:
Generating the following SQL:
Additional information
Several syntaxes have previously been discussed to support recursive CTEs, as well as unions.
.with(:recursive, …).with.recursive(…)(which this PR implements) and.with_recursive(…).with_recursive(cte_name, base_case, recursive_case)I took a lot of inspiration from that last monkey-patch, but the API felt significantly at odds with the existing
QueryMethods#with, so I decided to go with the.with_recursiveapproach and extend that interface to offer toUNION ALLsub-queries.I have decided to put both the array support and the
.with_recursivesupport in the same PR, as they are closely related, but I can split it into two PRs if preferred.Tests are updated accordingly, but don't actually test inherently recursive queries, as I'm not sure there are existing fixtures lending themselves to that.
Checklist
Before submitting the PR make sure the following are checked:
[Fix #issue-number]