Commit e2685352 authored by Dylan Griffith's avatar Dylan Griffith Committed by Stan Hu

Always sort Elasticsearch searches by relevance not updated_at

**TL;DR** Elasticsearch defaults to sorting by `_score` and this is what
we want so remove all the explicit sorting.

At present some searches are being sorted by `_score` (ie. the relevance
score) while others are being first sorted by `updated_at` then
`_score`.

There doesn't really appear to be any good reason to use `updated_at` as
the primary sort followed by `_score` since this will bury the most
relevant matches behind lots of recent matches. Using sort in this way
will only ever sort by `_score` if the `updated_at` was equal but this
is basically never going to happen since these are timestamps so
`_score` in practice is never factoring in to the sorting which probably
was never the intent here.

Since the [default sorting is by
score](https://www.elastic.co/guide/en/elasticsearch/guide/master/sorting.html)
we can leave it out of all queries.

We can in future look into whether or not we want recency to factor into
the sorting using more advanced tools like
[`function_score`](https://www.elastic.co/guide/en/elasticsearch/guide/current/function-score-query.html)
but for now sorting just by relevance is better than sorting just by
`updated_at`.
parent 46a91af8
---
title: Always sort Elasticsearch searches by relevance not updated_at
merge_request: 36774
author:
type: fixed
...@@ -71,11 +71,6 @@ module Elastic ...@@ -71,11 +71,6 @@ module Elastic
} }
end end
query_hash[:sort] = [
{ updated_at: { order: :desc } },
:_score
]
query_hash[:highlight] = highlight_options(fields) query_hash[:highlight] = highlight_options(fields)
query_hash query_hash
......
...@@ -92,8 +92,6 @@ module Elastic ...@@ -92,8 +92,6 @@ module Elastic
options[:order] = :default if options[:order].blank? options[:order] = :default if options[:order].blank?
query_hash[:sort] = [:_score]
res = search(query_hash, options) res = search(query_hash, options)
{ {
results: res.results, results: res.results,
...@@ -154,8 +152,6 @@ module Elastic ...@@ -154,8 +152,6 @@ module Elastic
options[:order] = :default if options[:order].blank? options[:order] = :default if options[:order].blank?
query_hash[:sort] = [:_score]
if options[:highlight] if options[:highlight]
query_hash[:highlight] = { query_hash[:highlight] = {
pre_tags: ["gitlabelasticsearch→"], pre_tags: ["gitlabelasticsearch→"],
......
...@@ -16,11 +16,6 @@ module Elastic ...@@ -16,11 +16,6 @@ module Elastic
query_hash = project_ids_filter(query_hash, options) query_hash = project_ids_filter(query_hash, options)
query_hash = confidentiality_filter(query_hash, options[:current_user]) query_hash = confidentiality_filter(query_hash, options[:current_user])
query_hash[:sort] = [
{ updated_at: { order: :desc } },
:_score
]
query_hash[:highlight] = highlight_options(options[:in]) query_hash[:highlight] = highlight_options(options[:in])
search(query_hash, options) search(query_hash, options)
......
...@@ -42,8 +42,6 @@ module Elastic ...@@ -42,8 +42,6 @@ module Elastic
query_hash[:query][:bool][:filter] = filters query_hash[:query][:bool][:filter] = filters
query_hash[:sort] = [:_score]
search(query_hash, options) search(query_hash, options)
end end
end end
......
...@@ -22,8 +22,11 @@ RSpec.describe Gitlab::Elastic::SnippetSearchResults, :elastic, :sidekiq_might_n ...@@ -22,8 +22,11 @@ RSpec.describe Gitlab::Elastic::SnippetSearchResults, :elastic, :sidekiq_might_n
end end
it 'returns the correct page of results' do it 'returns the correct page of results' do
expect(results.objects('snippet_titles', page: 1, per_page: 1)).to eq([snippet2]) # `snippet` is more relevant than `snippet2` (hence first in order) due
expect(results.objects('snippet_titles', page: 2, per_page: 1)).to eq([snippet]) # to having a shorter title that exactly matches the query and also due
# to having a description that matches the query.
expect(results.objects('snippet_titles', page: 1, per_page: 1)).to eq([snippet])
expect(results.objects('snippet_titles', page: 2, per_page: 1)).to eq([snippet2])
end end
it 'returns the correct number of results for one page' do it 'returns the correct number of results for one page' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment