Patch Review: #576 adds a :load option to ActiveRecord

I like the preloading functionality in ActiveRecord. Can we expose an
API option for it? I know it's important to be careful with adding AR
options, but I believe that adding a :load option would complement the
existing :include and :joins options as follows:

* :include names associations that should be eager loaded but may also
be needed for sql references (e.g. in conditions, orders, etc.)
* :joins names associations that are needed for sql references but
should not be loaded
* :load names associations that should be loaded but are not needed
for sql references

Comments and discussion appreciated! When I talked with Pratik in IRC,
his concern was adding another explicit option when perhaps more
intelligence could instead be applied to existing options. This is a
case where it seems cleaner to add another option. Thoughts?

-Lance

I'd quite like to be able to preload associations after the records
have been loaded (which is of course exactly what happens under the
hood anyway).

You get back some objects from something that has no idea what you're
going to do, but you know that in this case you're going to be
iterating over them and displaying the value of association X - it
would be neat to be able to say - I'm going to be looking at X - fetch
those objects (of course datamapper style automatic doing this could
potentially be even better).

I think this would avoid the adding of an extra option to the find
method and actually buy you some flexability since it divorces the
notions of loading the records and preloading an association(s)

Fred

You can actually call the preload_associations method whenever you
want. It's not public, though, so you need to use send() -- perhaps
that should change?

You can actually call the preload_associations method whenever you
want.

I know, I wrote it :slight_smile:

It's not public, though, so you need to use send() -- perhaps
that should change?

Something along those lines maybe some tidying up if it was to be for
public consumption.
on association proxies you could do something like
post.comments.load(:author => :avatar). but for a Foo.find we can't do
that yet since we don't have a proxy in that case.

Fred

Something along those lines maybe some tidying up if it was to be for
public consumption.
on association proxies you could do something like
post.comments.load(:author => :avatar). but for a Foo.find we can't do
that yet since we don't have a proxy in that case.

Something like this could be very nice, though the ideal would be if
AR figured out what you wanted and preloaded lazily. Though obviously
this requires the notion of a result set and all the associated
classes. If someone wants to work on this it could be a really nice
enhancement.

> You can actually call the preload_associations method whenever you
> want.

I know, I wrote it :slight_smile:

Hey thanks! Love it. :slight_smile:

> It's not public, though, so you need to use send() -- perhaps
> that should change?

Something along those lines maybe some tidying up if it was to be for
public consumption.
on association proxies you could do something like
post.comments.load(:author => :avatar). but for a Foo.find we can't do
that yet since we don't have a proxy in that case.

I do like that notation. In my case I've got quite a few associations
that I'd like to have preloaded, so being able to pass arrays/hashes
to preload_associations (well, indirectly through the :load option)
make it much simpler.

Another benefit of a :load option for find is the ability to merge
scopes. I actually like to set up named_scopes in the format
"for_#{view-name}"; these named_scopes tack on all the association
loading necessary for that view to render without n+1 problems (or 3n
+1 even). With :load, I can make that happen without worrying about
side effects on the result set itself.

Could we get away with find :all returning a proxy that looked enough like an array that it wouldn't break everyone's apps ?

Fred

Could we get away with find :all returning a proxy that looked enough
like an array that it wouldn't break everyone's apps ?

Probably, but it's probably a fairly large amount of work. If people
want to work on it then I'm happy to help provide guidance and the
like, but it's probably going to take till 2.3 till we could have
something like this land.

That's definitely the right approach but it's not going to be
s/Array/NewFancyProxy/g :wink:

Well I'm up for a challenge :slight_smile:

Was just playing around with things and adding

def records.load associations
   self.first.class.send :preload_associations, self, associations unless self.empty?
   self
end

to find_every allows you to do stuff like

conversations = Conversation.find :all
conversations.load :messages => :author

It's not hard to add this to association collection either. I don't think this approach can be taking any further and it does mean that you can't Marshal.dump what you get out of a find (not sure how much of an issue this is but there are tests around that area).

I may just be being prematurely paranoid but if find returns a result set object and if the objects in that collection know the result set object (so that for example accessing an association can preemptively load that association for all of them) would you not have the problem that ruby would hold onto the entire result set as long as anyone was holding onto any single object in that collection ?

Fred

It's not hard to add this to association collection either. I don't
think this approach can be taking any further and it does mean that
you can't Marshal.dump what you get out of a find (not sure how much
of an issue this is but there are tests around that area).

I'd definitely prefer to use a class we defined rather than the
metaclass approach, but this is the kind of thing you'll need to do.

I may just be being prematurely paranoid but if find returns a result
set object and if the objects in that collection know the result set
object (so that for example accessing an association can preemptively
load that association for all of them) would you not have the problem
that ruby would hold onto the entire result set as long as anyone was
holding onto any single object in that collection ?

No you're not being paranoid, if the objects can reach their
collection (and the have to be able to) then the whole collection will
stay in memory while any instance is. ORMs which use this approach
and have large user bases typically have things like the notion of a
'detached' object. In this case user's would have to detach those
objects.

Or you could hold the association to the collection in a WeakRef, but
I'm not sure how well tested or widely used that class is right now...

There could be some other approach I'm forgetting, but it seems to me
it's a restriction you'd have to keep in mind.

It's not hard to add this to association collection either. I don't
think this approach can be taking any further and it does mean that
you can't Marshal.dump what you get out of a find (not sure how much
of an issue this is but there are tests around that area).

I'd definitely prefer to use a class we defined rather than the
metaclass approach, but this is the kind of thing you'll need to do.

Sure, just thought it was a fun little hack.

I may just be being prematurely paranoid but if find returns a result
set object and if the objects in that collection know the result set
object (so that for example accessing an association can preemptively
load that association for all of them) would you not have the problem
that ruby would hold onto the entire result set as long as anyone was
holding onto any single object in that collection ?

No you're not being paranoid, if the objects can reach their
collection (and the have to be able to) then the whole collection will
stay in memory while any instance is. ORMs which use this approach
and have large user bases typically have things like the notion of a
'detached' object. In this case user's would have to detach those
objects.

Or you could hold the association to the collection in a WeakRef, but
I'm not sure how well tested or widely used that class is right now...

Thanks for the confirmation. I'll probably try and give this a go as a
plugin ( if it doesn't end up being too much hassle) as it really
sounds like something that would benefit from people being able to
play with it easily

Fred

Thanks for the confirmation. I'll probably try and give this a go as a
plugin ( if it doesn't end up being too much hassle) as it really
sounds like something that would benefit from people being able to
play with it easily

Yeah it'd be great to be able to get this into people's hands to
figure out the real world benefits and costs of this kind of change.
It's a lovely theoretical improvement, it'd be nice to validate that.
If it gets too cumbersome to hack around, we could add some hooks
for the plugin to work with.