github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-05-22 22:43:33 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

FIX: Select earliest post when aggregating posts in a topic for search.

Browse Source 
This is a revert of
d8c796bc44
and
5bf0a0893b.

Linking to the post within a topic that has the highest rank was
confusing users and hard to explain because ranking is determined via
the PG ranking function. See the following meta topics for the
complaints after we switch to the new ordering:

1. https://meta.discourse.org/t/title-search-not-working-as-expected/157737
2. https://meta.discourse.org/t/search-results-should-prioritize-first-post-in-topic-when-title-matches-search-term/175154
This commit is contained in:
Alan Guo Xiang Tan
2021-02-01 13:40:06 +08:00
parent 91fb298510
commit 4b3f65bb26
2 changed files with 49 additions and 87 deletions
Download Patch File Download Diff File Expand all files Collapse all files

108
lib/search.rb
View File

@ -986,53 +986,30 @@ class Search
posts posts
end end
if aggregate_search
aggregate_relation = Post.unscoped
.select("subquery.topic_id id")
.group("subquery.topic_id")
posts = posts.select(posts.arel.projections)
end
if @order == :latest if @order == :latest
posts = posts.reorder("posts.created_at DESC")
if aggregate_search if aggregate_search
aggregate_relation = aggregate_relation posts = posts.order("MAX(posts.created_at) DESC")
.select( else
"MAX(subquery.post_number) post_number", posts = posts.reorder("posts.created_at DESC")
"MAX(subquery.created_at) created_at"
)
.order("created_at DESC")
end end
elsif @order == :latest_topic elsif @order == :latest_topic
posts = posts.order("topics.created_at DESC")
if aggregate_search if aggregate_search
posts = posts.select("topics.created_at topic_created_at") posts = posts.order("MAX(topics.created_at) DESC")
else
aggregate_relation = aggregate_relation posts = posts.order("topics.created_at DESC")
.select(
"MIN(subquery.post_number) post_number",
"MAX(subquery.topic_created_at) topic_created_at"
)
.order("topic_created_at DESC")
end end
elsif @order == :views elsif @order == :views
posts = posts.order("topics.views DESC")
if aggregate_search if aggregate_search
posts = posts.select("topics.views topic_views") posts = posts.order("MAX(topics.views) DESC")
else
aggregate_relation = aggregate_relation posts = posts.order("topics.views DESC")
.select(
"MIN(subquery.post_number) post_number",
"MAX(subquery.topic_views) topic_views"
)
.order("topic_views DESC")
end end
elsif @order == :likes elsif @order == :likes
if aggregate_search
posts = posts.order("MAX(posts.like_count) DESC")
else
posts = posts.order("posts.like_count DESC") posts = posts.order("posts.like_count DESC")
end
else else
rank = <<~SQL rank = <<~SQL
TS_RANK_CD( TS_RANK_CD(
@ -1069,30 +1046,21 @@ class Search
"(#{rank} * #{category_priority_weights})" "(#{rank} * #{category_priority_weights})"
end end
posts =
if aggregate_search if aggregate_search
posts = posts.select("#{data_ranking} rank", "topics.bumped_at topic_bumped_at") posts.order("MAX(#{data_ranking}) DESC")
.order("rank DESC", "topic_bumped_at DESC")
aggregate_relation = aggregate_relation
.select(
"(ARRAY_AGG(subquery.post_number ORDER BY subquery.rank DESC, subquery.topic_bumped_at DESC))[1] post_number",
"MAX(subquery.rank) rank", "MAX(subquery.topic_bumped_at) topic_bumped_at"
)
.order("rank DESC", "topic_bumped_at DESC")
else else
posts = posts.order("#{data_ranking} DESC", "topics.bumped_at DESC") posts.order("#{data_ranking} DESC")
end
end end
posts = posts.order("topics.bumped_at DESC")
end
posts =
if secure_category_ids.present? if secure_category_ids.present?
posts = posts.where("(categories.id IS NULL) OR (NOT categories.read_restricted) OR (categories.id IN (?))", secure_category_ids).references(:categories) posts.where("(categories.id IS NULL) OR (NOT categories.read_restricted) OR (categories.id IN (?))", secure_category_ids).references(:categories)
else else
posts = posts.where("(categories.id IS NULL) OR (NOT categories.read_restricted)").references(:categories) posts.where("(categories.id IS NULL) OR (NOT categories.read_restricted)").references(:categories)
end
if aggregate_search
posts = yield(posts) if block_given?
posts = aggregate_relation.from(posts)
end end
if @order if @order
@ -1169,37 +1137,29 @@ class Search
Search.min_post_id Search.min_post_id
end end
min_or_max = @order == :latest ? "max" : "min"
query =
if @order == :likes if @order == :likes
# likes are a pain to aggregate so skip # likes are a pain to aggregate so skip
query = posts_query(limit, **default_opts).select('topics.id', 'posts.post_number') posts_query(limit, type_filter: opts[:type_filter])
.select('topics.id', "posts.post_number")
else
posts_query(limit, aggregate_search: true, type_filter: opts[:type_filter])
.select('topics.id', "#{min_or_max}(posts.post_number) post_number")
.group('topics.id')
end
if min_id > 0 if min_id > 0
low_set = query.dup.where("post_search_data.post_id < #{min_id}") low_set = query.dup.where("post_search_data.post_id < #{min_id}")
high_set = query.where("post_search_data.post_id >= #{min_id}") high_set = query.where("post_search_data.post_id >= #{min_id}")
{ default: wrap_rows(high_set), remaining: wrap_rows(low_set) } return { default: wrap_rows(high_set), remaining: wrap_rows(low_set) }
else
{ default: wrap_rows(query) }
end
else
query = posts_query(limit, **default_opts, aggregate_search: true) do |posts|
if min_id > 0
posts.select("post_search_data.post_id post_search_data_post_id")
else
posts
end
end end
if min_id > 0 # double wrapping so we get correct row numbers
low_set = query.dup.where("subquery.post_search_data_post_id < #{min_id}")
high_set = query.where("subquery.post_search_data_post_id >= #{min_id}")
{ default: wrap_rows(high_set), remaining: wrap_rows(low_set) }
else
{ default: wrap_rows(query) } { default: wrap_rows(query) }
end end
end
end
def aggregate_posts(post_sql) def aggregate_posts(post_sql)
return [] unless post_sql return [] unless post_sql

10
spec/components/search_spec.rb
View File

@ -601,8 +601,8 @@ describe Search do
expect(result.posts.pluck(:id)).to eq([post2.id, post.id]) expect(result.posts.pluck(:id)).to eq([post2.id, post.id])
end end
it 'aggregates searches in a topic by returning the post with the highest rank' do it 'aggregates searches in a topic by returning the post with the lowest post number' do
_post = Fabricate(:post, topic: topic, raw: "this is a play post") post = Fabricate(:post, topic: topic, raw: "this is a play post")
post2 = Fabricate(:post, topic: topic, raw: "play play playing played play") post2 = Fabricate(:post, topic: topic, raw: "play play playing played play")
post3 = Fabricate(:post, raw: "this is a play post") post3 = Fabricate(:post, raw: "this is a play post")
@ -613,7 +613,7 @@ describe Search do
results = Search.execute('play') results = Search.execute('play')
expect(results.posts.map(&:id)).to eq([ expect(results.posts.map(&:id)).to eq([
post2.id, post.id,
post3.id post3.id
]) ])
end end
@ -1892,9 +1892,11 @@ describe Search do
it 'allows to define custom order' do it 'allows to define custom order' do
expect(Search.new("advanced").execute.posts).to eq([post1, post0]) expect(Search.new("advanced").execute.posts).to eq([post1, post0])
Search.advanced_order(:chars) do |posts| Search.advanced_order(:chars) do |posts|
posts.reorder("(SELECT LENGTH(raw) FROM posts WHERE posts.topic_id = subquery.topic_id) DESC") posts.reorder("MAX(LENGTH(posts.raw)) DESC")
end end
expect(Search.new("advanced order:chars").execute.posts).to eq([post0, post1]) expect(Search.new("advanced order:chars").execute.posts).to eq([post0, post1])
end end
end end