Elasticsearch-rails: ActiveRecord collection not (obviously) sorted by score

[alexceder]

All this came about when updating from Rails 4.2 to Rails 5.

I have hade some trouble with Elasticsearch not returning ActiveRecord models in order of score.

Reviewing the response, I can see that they are correctly returned in order by score so that:

response.map(&:id)
# => ["13", "12"]

However this is not what I want:

response.records.map(&:id)
# => [12, 13]

And this is where you might thing.. well you need to call to_a on the records. And yes, that works.

response.records.to_a.map(&:id)
# => [13, 12]

Just as the README states, it runs the corresponding SQL with only a WHILE statement. This confused me, where do you sort!?

https://github.com/elastic/elasticsearch-rails/blob/master/elasticsearch-model/README.md#search-results-as-database-records

The sorting is done when explicitly calling to_a. This is of course fine, but this isn't the big thing that confused me or where my errors appeared.

I first used this solution, but I thought it was rather ugly. This way I get the records sorted properly from the database, on primary key and not undefined behaviour.

response.records.order("FIELD (`table_name`.`primary_key`, 13, 12)").to_a.map(&:id)
# => [13, 12]

But scrap that, I don't want to write custom SQL for which should happen automagically.

So why am I writing this loooong wall of text.. Well, it seems like Rails has changed how it uses implicit convertions.. or something.. so.. This is probably where something happens (or more likely doesn't happen):

# Wrong ordering
render json: records

Because if I explicitly convert to array, all is well.

# Correct ordering
render json: records.to_a

This might be more of a gotcha, but it was a pain for me, and might be for others.

Is there a better way of doing this? Some other intended use? Is this dumb? Could the "default" order be done like I did it? Ideas? Dumb?

[johanlunds]

Possibly related:

• https://github.com/elastic/elasticsearch-rails/issues/258
• https://github.com/elastic/elasticsearch-rails/issues/594
• https://github.com/elastic/elasticsearch-rails/issues/546
• https://github.com/elastic/elasticsearch-rails/issues/169
• https://github.com/elastic/elasticsearch-rails/issues/568

[dimroc]

I also upgraded from Rails 4.2 to Rails 5 and ran into the same sorting issue. I explicitly had a sort term in the ES query using the elasticsearch dsl. The ES response was correctly sorted, but when using records the order was ignored.

[dimroc]

As a stop gap, I have been using this custom ElasticRelation class to wrap around records:

rows = ElasticRelation.new Row.search(params).records

It essentially sorts in memory as originally implemented here:

https://github.com/elastic/elasticsearch-rails/blob/master/elasticsearch-model/lib/elasticsearch/model/adapters/active_record.rb#L35:

  def sorted_by_score
    hits = records.response.response.hits.hits
    records.sort_by do |record|
      hits.index { |hit| hit['_id'].to_s == record.id.to_s }
    end
  end

Figuring out how to plug this into Rails 5 AR is the fun part.

[inkstak]

AR 5 delegates array methods on records instead of to_a

Delegation: https://github.com/rails/rails/blob/master/activerecord/lib/active_record/relation/delegation.rb#L39

records method: https://github.com/rails/rails/blob/master/activerecord/lib/active_record/relation.rb#L257

I guess the name of the singleton method redefined by elasticsearch-models should depends on ActiveRecord version

[sr3d]

Confirmed about the bug happening for Rails 5 upgrading from Rails 4.2

[blackst0ne]

Any progress on this?

[johanlunds]

I think it has been merged. Read the comments on the referenced PR.

[karmi]

First of all, my apologies, @alexceder, for not responding to this issue sooner, and many thanks for the care you took in providing all the details in the issue description. Also, many thanks for everyone responding on this thread!

As @johanlunds says, the fix by @inkstak has indeed been merged through pull request https://github.com/elastic/elasticsearch-rails/pull/618.

To reply to the original question by @alexceder, I do agree that the whole story about needing to call to_a on the records object has been confusing, and I can imagine that _many_ people have walked into that trap. However, at the time I couldn't figure out any other way how to do it, because I wanted the records object to be a true instance of ActiveRecord::Relation, so you can call eg. join on it, or do any other SQL tricks. I can imagine there was such a way, but I simply wasn't able to find it.

@alexceder, @sr3d, can you check your code with the current master, which contains the patch, and verify that it works as expected, and that there's less confusion about it in the application code? If you'd find any discrepancies or problems, I'd like to add tests for them in https://github.com/elastic/elasticsearch-rails/blob/master/elasticsearch-model/test/integration/active_record_basic_test.rb#L108-L121 to be able to fix them in an organized manner.

[jujudellago]

I'm not sure if this was fixed for ActiveRecord, but I have exactly the same issue with Mongoid

[arcifius]

I'm also having this issue with Mongoid ;/

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.