Preserve RECURSIVE flag of CTE relations when they are merged.#189
Open
jsnelling wants to merge 1 commit intoDavyJonesLocker:masterfrom
Open
Preserve RECURSIVE flag of CTE relations when they are merged.#189jsnelling wants to merge 1 commit intoDavyJonesLocker:masterfrom
jsnelling wants to merge 1 commit intoDavyJonesLocker:masterfrom
Conversation
Queries like the following:
Tag.all.merge(Tag.with.recursive(recursive: Tag.where(tag: 'tag')))
Tag.with.recursive(recursive: Tag.where(tag: 'tag')).merge(Tag.all)
would both be expected to produce a query like:
WITH RECURSIVE "recursive"
AS ( SELECT "tags".*
FROM "tags"
WHERE "tags"."tag" = 'tag')
SELECT "tags".*
FROM "tags"
but currently only the second one would. The first would lose the recursive flag
during merge and result in:
WITH "recursive"
AS ( SELECT "tags".*
FROM "tags"
WHERE "tags"."tag" = 'tag')
SELECT "tags".*
FROM "tags"
Adding :recursive to ActiveRecord::Relation::Merger#normal_values
and a #recursive! method to ActiveRecord::QueryMethods the expected behavior
regardless of the direction of the merge.
Collaborator
|
Thank you @jsnelling I will review this weekend and hopefully merge |
Author
|
hey @edpaget, I wanted to check in and see if there was anything else that you needed from me on this PR? |
|
:( |
Author
|
@Papipo unfortunately, it doesn't seem that this project is active anymore |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request fixes what I think to be a bug in the handling of recursive CTEs during merges.
Queries like the following:
Tag.all.merge(Tag.with.recursive(recursive: Tag.where(tag: 'tag')))
Tag.with.recursive(recursive: Tag.where(tag: 'tag')).merge(Tag.all)
would both be expected to produce a query like:
WITH RECURSIVE "recursive"
AS ( SELECT "tags".*
FROM "tags"
WHERE "tags"."tag" = 'tag')
SELECT "tags".*
FROM "tags"
but currently only the second one would. The first would lose the recursive flag
during merge and result in:
WITH "recursive"
AS ( SELECT "tags".*
FROM "tags"
WHERE "tags"."tag" = 'tag')
SELECT "tags".*
FROM "tags"
Adding :recursive to ActiveRecord::Relation::Merger#normal_values
and a #recursive! method to ActiveRecord::QueryMethods the expected behavior
regardless of the direction of the merge.
Closes # .
Changes proposed in this pull request