Sounds good, we'll do that, thanks. Kind regards, Janko
On Thu, Oct 25, 2018 at 4:18 PM Jeremy Evans <[email protected]> wrote: > On Thursday, October 25, 2018 at 2:07:39 AM UTC-7, Janko Marohnić wrote: >> >> Hey everyone, >> >> In our application N+1 queries often creep in due to associations being >> loaded dynamically. Since they keep coming back, I would like to take it >> one step further and make Sequel raise an exception when an association was >> requested but hasn't been eager loaded. >> >> I know about the tactical_eager_loading >> <http://sequel.jeremyevans.net/rdoc-plugins/classes/Sequel/Plugins/TacticalEagerLoading.html> >> plugin, >> but I want to always make it explicit which associations are being loaded, >> I don't want it to be implicit. >> >> I was thinking that my "strict eager loading" plugin would behave like >> the following: >> >> There would be a dataset method, e.g. `#require_eager_loading`, which you >> can call before retrieving records. When that dataset loads model >> instances, attempting to call association retrieval methods on those >> instances will fail if the associations haven't been eager loaded (i.e. if >> Sequel::Model#associations cache doesn't contain the corresponding loaded >> association). The reasons for making this behaviour opt-in via a dataset >> method, as opposed to having it be global activated, is so that (a) you can >> gradually get your codebase rid of N+1s, and (b) you can still dynamically >> load associations in tests, as having to eager load associations in tests >> would probably add unnecessary verbosity. >> >> The model instances of associations eager loaded from a "strict eager >> loading" enabled dataset should also be required to eager load *their* >> associations. >> In other words, the option that would be set by `#require_eager_loading` >> should "spread" onto the model instances of associations as well. >> >> The way we would would slowly migrate to "strict eager loading" mode in >> our app is that we would start making queries through our query objects (as >> opposed to through Sequel::Model or Sequel::Database). These query objects >> would then automatically add `#require_eager_loading` to each dataset. That >> way retrieving data through Sequel::Model or Sequel::Database would still >> allow dynamic loading of associations, but retrieving data through custom >> query objects wouldn't. >> >> What do you think about this idea? I was thinking that we would first >> write such a plugin internally in our app to try it out, and then I would >> probably propose it to Sequel. I think it would be a great asset to Sequel >> if it was able to disallow N+1 queries like that. >> > > I think it would be best if you would write the plugin internally, then > use it for 3-6 months in production. After that time, if you think it is > generally useful, then ask here if you would like it to be included with > Sequel. Personally, I'd call the plugin disallow_lazy_association_loading > or something similar to make it obvious that the behavior it changes is > disallowing lazy association loading, as it sounds like it would not affect > eager association loading. > > Thanks, > Jeremy > > -- > You received this message because you are subscribed to a topic in the > Google Groups "sequel-talk" group. > To unsubscribe from this topic, visit > https://groups.google.com/d/topic/sequel-talk/3g4CNvpJSiI/unsubscribe. > To unsubscribe from this group and all its topics, send an email to > [email protected]. > To post to this group, send email to [email protected]. > Visit this group at https://groups.google.com/group/sequel-talk. > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "sequel-talk" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at https://groups.google.com/group/sequel-talk. For more options, visit https://groups.google.com/d/optout.
