[MODEL] Add the :includes option to ActiveRecord adapter

[dabit]

that can be used to eager load submodels just like
you'd do with regular ActiveRecord.

This helps eliminate the N+1 problem when using
elasticsearch-model searches.

Example:

class Article < ActiveRecord::Base
  belongs_to :author

  # ...

end

Article.search(...).records(includes: [:author])

# Would be similar to using:

Article.includes(:author).where(...)

There might be implications that I'm not considering for other adapters but it works for me with ActiveRecord and all tests are green.

[karmi]

It definitely makes sense to nicely support the behaviour, I'm just wondering, since records is a true ActiveRecord::Relation instance, isn't it possible to just call eg. Category.search('one').records.includes(:articles).to_a -- if I work with the code in the associations example?

[niuage]

@karmi

You can technically call includes on records, but here's my issue:

records = Search::MapSearch.new(params).search.page(page).per(per_page).records
records.total_count
# => 740

records = Search::MapSearch.new(params).search.page(page).per(per_page).records.includes(map_items: :item)
records.total_count
# undefined method `total_count' for #<Map::ActiveRecord_Relation:0x007fd968e50878>

I guess I could do

records = Search::MapSearch.new(params).search.page(page).per(per_page).records
total_count = records.total_count
records = records.includes(map_items: :item)

It seems like it works fine, but that's not ideal.

[niuage]

Actually, it's not working as I expected...

search = ES::MapSearch.new(params).search
@total_count = search.total_count # performs the ES query
@took = search.took

@maps = search.page(page).per(per_page).records
@maps = @maps.includes(:user, { map_items: :item }) # performs the ES query a second time! 

The Elasticsearch query is performed twice!
I'm not sure how to get pagination, includes, and get some raw data about the query (took).

[donce]

Having same issue, this PR would help a lot!

[donce]

I've used this approach - but still, elasticsearch could support this feature, since approach I've used is hacky.

[niuage]

That seems like a pretty nice solution, well done. What I had done personally is just give up on kaminari pagination, using size and from in the ES query, and then getting the models via the result ids, since I just needed a "Load more" type of pagination.

[brianstorti]

Hi, @karmi. Any plans to get this merged?

[karmi]

My apologies to all, and first of all to you, @dabit, this took too long. Thanks for pinging me, @brianstorti!

I've added an integration test in the attached commit, and merged. I wanted to explore if we couldn't fix the more natural way of expressing the same, Category.search('one').records.includes(:articles).to_a, but I couldn't put much time into it, so let's start with merging @dabit's patch, and maybe work further in the future.

Thanks for the link to the decorator article, @donce!

[dabit]

@karmi no worries, this is great timing as precisely yesterday I had to patch one of my apps with this change and I did get a considerable performance boost. When there's a lot of data showing up in a page from different related models, that N+1 situation can kill you.

Thanks!

[brianstorti]

Awesome. Thanks, @karmi and @dabit. Is this going to be released soon?

[karmi]

@brianstorti, I'd like to accumulate more changes before a release to Rubygems, but if you need a release for your environment and cannot just use Git in Gemfile, I can do a release.

[karmi]

Actually, there has been a lot accumulated already :), so just released 0.1.9. There's some kind of weird unit test failures depending on whole test suite running or not, which I investigated for a bit, but didn't chew through it, will have a look at it later.

[michaelrshannon]

Was excited to find this PR, but having some issues implementing:
Gemfile

gem 'elasticsearch-rails', '~> 0.1.9'

/models/product.rb

  has_many :product_images, dependent: :destroy

controller

results = Product.search(query)
results = results.page(params[:page]).records(includes: [:product_images])

I get an argument error: wrong number of arguments (1 for 0)

Have I misunderstood how to implement this? Thanks in advance!

[niuage]

I don't think you need to call results if you're calling records. What about just Product.search(query).page(...).records(includes: [:product_images]).