Preserving has_many :through ids assignment order
dasil003
is that the order is not preserved. The definition looks like this:
define_method("#{reflection.name.to_s.singularize}_ids=") do |
new_value|
ids = (new_value || []).reject { |nid| nid.blank? }
send("#{reflection.name}=", reflection.klass.find(ids))
end
I worked up a fix for my particular case by redefining this directly
on my model with something like this:
define_method("#{reflection.name.to_s.singularize}_ids=") do |
new_value|
ids = (new_value || []).reject { |nid| nid.blank? }
hash = reflection.klass.find(ids).inject({}){ |h,obj| h[obj.id] =
obj; h }
objects = ids.map{ |id| hash[id.to_i] }
send("#{reflection.name}=", objects)
end
Basically just resorting the objects rather than the default order
which comes out of the the standard AR#find method.
It occurs to me that this is desirable behavior, but obviously there
are performance implications here. On one hand it seems wasteful for
cases where its unnecessary, but on the other hand, building a shallow
hash should not be significant next to the DB insert. Since the items
come in in an arbitrary order there's no way to get them out of the DB
in order already.
Does anyone think this would belong in core or should I just keep it
as a special case in my model?
Matt Jones
On Nov 10, 2009, at 6:11 PM, dasil003 wrote:
> Basically just resorting the objects rather than the default order
> which comes out of the the standard AR#find method.
>
> It occurs to me that this is desirable behavior, but obviously there
> are performance implications here. On one hand it seems wasteful for
> cases where its unnecessary, but on the other hand, building a shallow
> hash should not be significant next to the DB insert. Since the items
> come in in an arbitrary order there's no way to get them out of the DB
> in order already.
>
> Does anyone think this would belong in core or should I just keep it
> as a special case in my model?
What's the use case for this? The objects aren't going to come out of
the DB in order the next time you load the object unless you're
putting them in order with options on the association that you haven't
shown here.
--Matt Jones
dasil003
going through.
In MySQL incidentally will usually give you this by default.
Matt Jones
On Nov 10, 2009, at 7:01 PM, dasil003 wrote:
>
> :order => 'id ASC' on the association that the has_many :through is
> going through.
>
> In MySQL incidentally will usually give you this by default.
"Usually" being the key word. In your case, it isn't doing that, as
all that the find() call is doing is generating a query like:
SELECT * FROM table_name WHERE id IN (x,y,z,etc)
If those records aren't coming back in the order you expect, that's
not Rails's fault...
--Matt Jones
dasil003
thinking, but you are clearly not understanding the issue. This is
not my own method, it is a method generated by the has_many :through
macro. Consider this concrete example:
Person has_many :routes
Person has_many :destinations, :through => :routes
Now say I do, @person.desination_ids = [5, 8, 3]
That is what the included method definition does. It's totally
reasonable to expect the first route insertion to be for destination
5, the second insertion to be for destination 8 and the third
insertion to be for desination 3. However it doesn't do this as you
pointed out due to the fact that it finds all the destinations without
any order. However as I tried to point out in the original post,
there is NO WAY to maintain the order in the SQL because it's
arbitrary, it's not sorted on any field.
That is why I redefined the method so that it uses the original array
to inform the final order of objects that get passed to the
association method (ie. @person.destinations). As you can see, the
ordering of my association is neither here nor there. It's the order
of insertion that matters, and it's not controlled by an order clause.
My question was not how do I solve this, my question was whether
people think that this should be submitted as a Rails patch. Having
thought about it further I'm convinced it should be.
Matt Jones
On Nov 12, 2009, at 11:54 AM, dasil003 wrote:
>
> Matt, your comment doesn't make any sense. I'm not sure what you are
> thinking, but you are clearly not understanding the issue.
Whoops - looks like I was reading the question the other way around
(that you *wanted* the records to come back in order). Sorry about that!
> This is
> not my own method, it is a method generated by the has_many :through
> macro. Consider this concrete example:
>
> Person has_many :routes
> Person has_many :destinations, :through => :routes
>
> Now say I do, @person.desination_ids = [5, 8, 3]
>
> That is what the included method definition does. It's totally
> reasonable to expect the first route insertion to be for destination
> 5, the second insertion to be for destination 8 and the third
> insertion to be for desination 3. However it doesn't do this as you
> pointed out due to the fact that it finds all the destinations without
> any order. However as I tried to point out in the original post,
> there is NO WAY to maintain the order in the SQL because it's
> arbitrary, it's not sorted on any field.
>
> That is why I redefined the method so that it uses the original array
> to inform the final order of objects that get passed to the
> association method (ie. @person.destinations). As you can see, the
> ordering of my association is neither here nor there. It's the order
> of insertion that matters, and it's not controlled by an order clause.
>
> My question was not how do I solve this, my question was whether
> people think that this should be submitted as a Rails patch. Having
> thought about it further I'm convinced it should be.
For this to make sense as a Rails patch, there's got to be a reason
why this ordering should be preserved. What's the difference between
adding the records in the [5,8,3] order vs. some other order?
--Matt JOnes
dasil003
> For this to make sense as a Rails patch, there's got to be a reason
> why this ordering should be preserved. What's the difference between
> adding the records in the [5,8,3] order vs. some other order?
it's creating records, and the order they are created in is
significant. Suppose this is populated by an javascript autocomplete
where a user is picking a series of destinations. If the declaration
is:
has_many :routes, :order => 'id'
Then the order that the routes are inserted is significant.
Now it's true that you can control the order by creating routes
manually, but being able to assign to the leaf association is
incredibly useful in certain circumstances because it can remove a ton
of boilerplate bookkeeping code. The view and controller don't even
need to know about Routes at all, they just work with Destinations
which is much cleaner.
Finally, I the order should be preserved for @person.destination_ids=
for simple symmetry with @person.destinations= and other iterating
object creators which all seem to preserve order in my experience.
Patch forthcoming...
Rick DeNatale
It seem to me that ordering on id isn't what you really want here.
What happens when you add the function to reorder the routes? If it
were me I'd probably do something like adding a position attribute to
route and use something like acts_as_list
--
Rick DeNatale
Blog: http://talklikeaduck.denhaven2.com/
Twitter: http://twitter.com/RickDeNatale
WWR: http://www.workingwithrails.com/person/9021-rick-denatale
LinkedIn: http://www.linkedin.com/in/rickdenatale
dasil003
seemingly vanished into thin air. I won't repeat the whole thing.
Essentially, my response was that, no Rick, I definitely want the
routes ordered by primary key, and depending on that order GREATLY
simplifies both my controller and my module (see http://www.theauteurs.com/lists
for an example). Acts_as_list is tremendous overkill in most
situations. Depending on insertion order simplifies a great many
things. With regards to ordering routes you can do so entirely at the
client level with something like Prototype's Sortable, and the
controller doesn't need to be aware of routes at all, it can use
purely desination ids which makes the resulting code extremely clean.
My patch is up on lighthouse, could use some reviewers:
https://rails.lighthouseapp.com/projects/8994/tickets/3491-assigning-to-has_many-through-using-the-generated-_ids-method-does-not-insert-in-the-same-order