I'm comfortable with those trade-offs. Barring any compelling real world code shining a different light on the discussion than what we've covered so far, I don't think we're going to make any additional progress discussing it.
On Thursday, July 16, 2015 at 3:12:56 PM UTC+2, will.bryant wrote: > > Oh, sorry, I explained that badly. > > I’m not saying it’s a performance problem doing two queries. I’m saying > it’s causing unintended side-effects, because when you do the query on the > parent it resets the attributes, including blowing away unsaved changes. > This is absolutely undesirable if you just want to make sure you have the > current child. > > How is it trivial to implement reload_manager on its own, BTW? You would > have to write out the foreign key SQL, do the query, and then assign back > to the association? > > > On 17/07/2015, at 01:07 , DHH <da...@loudthinking.com <javascript:>> > wrote: > > I'd be happy to consider some real code from a real project that shows the > use of reloading a single has_one association in a performance hotspot > where two ID lookups are proving to be a problem. But for the time being, > I'm content with the trade off that has collection#reload and record#reload > as the two only ways to reloading those associations. And if someone needs > #reload_manager in their model because they do use that a lot, it's trivial > to implement on its own. > > Do appreciate the debate over this, though! Good to go through the > reasoning of why we do and make certain changes. > > On Thursday, July 16, 2015 at 2:28:14 PM UTC+2, will.bryant wrote: >> >> I think you’re right that we can’t have complete parity, but then as you >> said, it would be good to have a single API. >> >> It does works well right now having the same API to always get the fresh >> association loaded, and it is useful to be able to definitely get the >> current record, even if you don’t personally use it that often :). >> >> There is a practical difference between the current project.manager(true) >> which always does exactly one query, whether or not manager is already >> loaded, and project.reload.manager, which always does two queries and also >> changes other stuff on the project instance. >> >> Surely it is not necessary to take away the ability to do exactly one >> association reload? >> >> Otherwise you could just as equally argue that there is no use for reload >> on a has_many. Personally I think I reload has_ones as often as I reload >> has_manys, and for the same reasons - I need to be sure I am dealing with >> the current child/children of an object I am halfway through doing stuff to >> (which is why I don’t want to reload the parent). >> >> >> On 17/07/2015, at 00:17 , DHH <da...@loudthinking.com> wrote: >> >> I think the first assumption to challenge is that we can have parity >> between a collection and a single object. I don't think we can or should. >> An array of strings does not have parity with a single string. >> >> So project.manager.reload will call ActiveRecord::Base#reload, so that's >> a Manager.find(id) call, not a Manager.find_by(project: x) call. If you >> absolutely need the latter, you can do project.reload.manager. I've found >> this to be an exceedingly rare case, though. So I'm find have it be a >> round-about to get there, and I'm fine not having parity between collection >> and record. >> >> On Thursday, July 16, 2015 at 2:09:22 PM UTC+2, will.bryant wrote: >>> >>> OK, so what about the has_one case? >>> >>> Say Project has_one :manager, and project.manager is already loaded >>> (which did a "SELECT * FROM managers WHERE project_id = ?"), will >>> project.manager.reload do another "SELECT * FROM managers WHERE project_id >>> = ?", or will it do a "SELECT * FROM managers WHERE id = ?”. >>> >>> To match the has_many I would expect the former, and sometimes we need >>> this behavior if this is the only way to force the association to be looked >>> up again (whereas currently we can do project.manager(true)). >>> >>> But because the object returned by project.manager looks like it is just >>> a plain record instance, I’d expect #reload to behave like the query, a >>> regular find(id). Especially if I call reload(:lock => true). >>> >>> It seems a big ambiguous what the behavior would be for has_one, which >>> makes me think the original project.manager(:reload => true) is clearer - >>> and then reload on the target of a belongs_to or has_one should always just >>> reload that instance, not load a potentially different record. >>> >>> Do you agree? >>> >>> >>> On 16/07/2015, at 23:59 , DHH <da...@loudthinking.com> wrote: >>> >>> project.documents.reload.first would reload the documents association, >>> then grab the first entry in that array – not trigger another find(id). >>> Like calling #load, but ignoring whether it had already been loaded. >>> >>> On Thursday, July 16, 2015 at 12:53:31 AM UTC+2, will.bryant wrote: >>>> >>>> So are you saying project.documents.reload.first would redo the query >>>> to find the associated record, or would do a find(id) to reload the >>>> current >>>> instance? >>>> >>>> >>>> On 16/07/2015, at 01:08 , DHH <da...@loudthinking.com> wrote: >>>> >>>> I'd prefer to see us move to a single API, and IMO the trade-offs for >>>> #reload alone fits the bill. There's no contract saying that a singular >>>> object from has_one/belongs_to and a collection like has_many has to >>>> behave >>>> the same. In fact, I think it'd be strange to think that it should. A >>>> single string and an array of strings do not behave the same. >>>> >>>> So project.documents.reload.first makes perfect sense to me as >>>> reloading the documents collection, then grabbing the first one. >>>> projects.owner.reload.name makes sense to me as reloading the owner >>>> record itself, then fetching the name (if we don't already return self >>>> from >>>> Record#reload, we should). >>>> >>>> -- >>>> You received this message because you are subscribed to the Google >>>> Groups "Ruby on Rails: Core" group. >>>> To unsubscribe from this group and stop receiving emails from it, send >>>> an email to rubyonrails-co...@googlegroups.com. >>>> To post to this group, send email to rubyonra...@googlegroups.com. >>>> Visit this group at http://groups.google.com/group/rubyonrails-core. >>>> For more options, visit https://groups.google.com/d/optout. >>>> >>>> >>>> >>> -- >>> You received this message because you are subscribed to the Google >>> Groups "Ruby on Rails: Core" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to rubyonrails-co...@googlegroups.com. >>> To post to this group, send email to rubyonra...@googlegroups.com. >>> Visit this group at http://groups.google.com/group/rubyonrails-core. >>> For more options, visit https://groups.google.com/d/optout. >>> >>> >>> >> -- >> You received this message because you are subscribed to the Google Groups >> "Ruby on Rails: Core" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to rubyonrails-core+unsubscr...@googlegroups.com <javascript:>. >> To post to this group, send email to rubyonra...@googlegroups.com. >> Visit this group at http://groups.google.com/group/rubyonrails-core. >> For more options, visit https://groups.google.com/d/optout. >> >> >> > -- > You received this message because you are subscribed to the Google Groups > "Ruby on Rails: Core" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to rubyonrails-core+unsubscr...@googlegroups.com <javascript:>. > To post to this group, send email to rubyonra...@googlegroups.com > <javascript:>. > Visit this group at http://groups.google.com/group/rubyonrails-core. > For more options, visit https://groups.google.com/d/optout. > > > -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscr...@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/d/optout.
