Fix: total_count with GROUPED STI relation #1015
Closed
Conversation
This file contains 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
When querying a subtype on a STI table, a 'hard scope' is applied that is incompatible with the use of `model.from(subquery)` This fix removes any type restriction from the _outer_ query (because the subquery already has the restriction). Introduced by kaminari#979
Incorporating solution kaminari#1012 which doesn't cover the special case of STI type restriction.
yuki24
added a commit
that referenced
this pull request
Feb 29, 2020
…or_grouped_queries" This reverts commit b5e2e07, reversing changes made to d1c26fd. We merged #979 as a performance improvement, but it turns out it introduced a couple of bugs. The solution seems to be a little tricky as it has to call `.unscoped` and `unscope(where: :type)`, so let's revert it until we find a better solution. closes #1012, #1015.
|
closed by 04d86ed |
|
I have reverted #979 and picked up the test from this PR. Thanks for reporting this issue! |
bendilley
added a commit
to skillstream/kaminari
that referenced
this pull request
May 13, 2020
Improved performance for total_count on grouped ActiveRecord::Relation. This re-implements kaminari#979 using Arel to avoid issues kaminari#1012 and kaminari#1015
bendilley
added a commit
to skillstream/kaminari
that referenced
this pull request
May 13, 2020
Improved performance for total_count on grouped ActiveRecord::Relation. This re-implements kaminari#979 using plain SQL to avoid issues kaminari#1012 and kaminari#1015
bendilley
added a commit
to skillstream/kaminari
that referenced
this pull request
May 13, 2020
Improved performance for total_count on grouped ActiveRecord::Relation. This re-implements kaminari#979 using Arel to avoid issues kaminari#1012 and kaminari#1015
This was referenced May 13, 2020
bendilley
added a commit
to skillstream/kaminari
that referenced
this pull request
May 29, 2020
Improved performance for total_count on grouped ActiveRecord::Relation. This re-implements kaminari#979 using plain SQL to avoid issues kaminari#1012 and kaminari#1015
bendilley
added a commit
to bendilley/kaminari
that referenced
this pull request
Oct 29, 2020
Improved performance for total_count on grouped ActiveRecord::Relation. This re-implements kaminari#979 using Arel to avoid issues kaminari#1012 and kaminari#1015
guigs
reviewed
Nov 3, 2020
| @@ -34,7 +34,8 @@ def total_count(column_name = :all, _options = nil) #:nodoc: | |||
|
|
|||
| # Handle grouping with a subquery | |||
| @total_count = if c.group_values.any? | |||
| c.model.from(c.except(:select).select("1")).count | |||
| # STI can place a 'hard scope' on the model which doesn't work on the outer query here | |||
| c.model.unscoped.unscope(where: :type).from(c.except(:select).select("1")).count | |||
There was a problem hiding this comment.
Better to use unscoped.unscope(:where) because one can use different column for STI (using inheritance_column)
There was a problem hiding this comment.
Further down @yuki24's "rabbit hole"!
Here's another idea which you could argue reads better than "unscope it and then unscope it some more":
Suggested change
| c.model.unscoped.unscope(where: :type).from(c.except(:select).select("1")).count | |
| c.model.base_class.unscoped.from(c.except(:select).select("1")).count |
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.
When querying a subtype on a STI table, a 'hard scope' is applied that
is incompatible with the use of
model.from(subquery)This fix removes any type restriction from the outer query (because
the subquery already has the restriction).
Introduced by #979