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

Merged
merged 1 commit into from Feb 24, 2013

Projects

None yet

2 participants

@senny
Member
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
Member
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.

@rafaelfranca rafaelfranca commented on the diff Feb 24, 2013
..._record/associations/preloader/through_association.rb
@@ -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])
@rafaelfranca
rafaelfranca Feb 24, 2013 Member

Why do we need this Array call?

@senny
senny Feb 24, 2013 Member

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

@rafaelfranca
Member

Seems good. Needs a rebase

@senny senny don't apply invalid ordering when preloading hmt associations.
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).
4ef75b6
@senny
Member
senny commented Feb 24, 2013

@rafaelfranca rebased.

@rafaelfranca rafaelfranca merged commit 99d3ec5 into rails:master Feb 24, 2013
@senny senny deleted the senny:8663_broken_hmt_ordering_with_includes branch Mar 6, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment