Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

don't apply invalid ordering when preloading hmt associations. #9391

Merged

Conversation

senny
Copy link
Member

@senny senny commented Feb 23, 2013

closes #8663.

When preloading a hmt association there two possible scenarios:

1.) preload with 2 queries: first hm association, then hmt with id IN ()
2.) preload with join: hmt association is loaded with a join on the hm association

The bug was happening in scenario 1.) with a normal order clause on the hmt association.
The ordering was also applied when loading the hm association, which resulted in the error.

This patch only applies the ordering the the hm-relation if we are performing a join (2).
Otherwise the order will only appear in the second query (1).

@senny
Copy link
Member Author

senny commented Feb 23, 2013

/cc @carlosantoniodasilva @rafaelfranca

@jonleighton as discussed, this patch is based on selectively adding the order wether we are preloading or not.

@@ -47,12 +47,12 @@ def through_scope
through_scope.where! reflection.foreign_type => options[:source_type]
else
unless reflection_scope.where_values.empty?
through_scope.includes_values = reflection_scope.values[:includes] || options[:source]
through_scope.includes_values = Array(reflection_scope.values[:includes] || options[:source])
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this Array call?

Copy link
Member Author

Choose a reason for hiding this comment

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

include_values must be an array. Otherwise the call to #eager_loading? will blow up.

@rafaelfranca
Copy link
Member

Seems good. Needs a rebase

closes rails#8663.

When preloading a hmt association there two possible scenarios:

1.) preload with 2 queries: first hm association, then hmt with id IN ()
2.) preload with join: hmt association is loaded with a join on the hm association

The bug was happening in scenario 1.) with a normal order clause on the hmt association.
The ordering was also applied when loading the hm association, which resulted in the error.

This patch only applies the ordering the the hm-relation if we are performing a join (2).
Otherwise the order will only appear in the second query (1).
@senny
Copy link
Member Author

senny commented Feb 24, 2013

@rafaelfranca rebased.

rafaelfranca added a commit that referenced this pull request Feb 24, 2013
…ludes

don't apply invalid ordering when preloading hmt associations.
@rafaelfranca rafaelfranca merged commit 99d3ec5 into rails:master Feb 24, 2013
@senny senny deleted the 8663_broken_hmt_ordering_with_includes branch March 6, 2013 15:19
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.

ActiveRecord has_many :through :order is broken when using includes
2 participants