mirror of
https://github.com/discourse/discourse.git
synced 2024-11-21 16:38:15 -06:00
PERF: Drop user_search_similar_results
site setting (#28874)
In 14cf8eacf1
, we added the
`user_search_similar_results` site setting which when enabled will use
trigram matching for similarity search in `UserSearch`. However, we
noted that adding the `index_users_on_username_lower_trgm` index is
causing the PG planner to not use the `index_users_on_username_lower`
index when the `=` operator is used against the `username_lower` column.
Based on the PG mailing list discussion where support for the `=`
operator in gist_trgm_ops was being considered, it stated that "I also have checked that btree_gist is preferred over pg_trgm gist
index for equality search." This is however quite different from reality
on our own PG clusters where the btree index is not preferred leading to
significantly slower queries when the `=` operator is used.
Since the pg_trgm gist index is only used for queries when the `user_search_similar_results` site setting
is enabled, we decided to drop the feature instead as it is hidden and
disabled by default. As such, we can consider it experiemental and drop
it without deprecation.
PG mailing list discussiong: https://www.postgresql.org/message-id/CAPpHfducQ0U8noyb2L3VChsyBMsc5V2Ej2whmEuxmAgHa2jVXg%40mail.gmail.com
This commit is contained in:
parent
c182bb34ad
commit
97143efc52
@ -2291,14 +2291,12 @@ end
|
||||
#
|
||||
# Indexes
|
||||
#
|
||||
# idx_users_admin (id) WHERE admin
|
||||
# idx_users_moderator (id) WHERE moderator
|
||||
# index_users_on_last_posted_at (last_posted_at)
|
||||
# index_users_on_last_seen_at (last_seen_at)
|
||||
# index_users_on_name_trgm (name) USING gist
|
||||
# index_users_on_secure_identifier (secure_identifier) UNIQUE
|
||||
# index_users_on_uploaded_avatar_id (uploaded_avatar_id)
|
||||
# index_users_on_username (username) UNIQUE
|
||||
# index_users_on_username_lower (username_lower) UNIQUE
|
||||
# index_users_on_username_lower_trgm (username_lower) USING gist
|
||||
# idx_users_admin (id) WHERE admin
|
||||
# idx_users_moderator (id) WHERE moderator
|
||||
# index_users_on_last_posted_at (last_posted_at)
|
||||
# index_users_on_last_seen_at (last_seen_at)
|
||||
# index_users_on_secure_identifier (secure_identifier) UNIQUE
|
||||
# index_users_on_uploaded_avatar_id (uploaded_avatar_id)
|
||||
# index_users_on_username (username) UNIQUE
|
||||
# index_users_on_username_lower (username_lower) UNIQUE
|
||||
#
|
||||
|
@ -172,27 +172,6 @@ class UserSearch
|
||||
.each { |id| users << id }
|
||||
end
|
||||
|
||||
return users.to_a if users.size >= @limit
|
||||
|
||||
# 6. similar usernames / names
|
||||
if @term.present? && SiteSetting.user_search_similar_results
|
||||
if SiteSetting.enable_names?
|
||||
scoped_users
|
||||
.where("username_lower <-> ? < 1 OR name <-> ? < 1", @term, @term)
|
||||
.order(["LEAST(username_lower <-> ?, name <-> ?) ASC", @term, @term])
|
||||
.limit(@limit - users.size)
|
||||
.pluck(:id)
|
||||
.each { |id| users << id }
|
||||
else
|
||||
scoped_users
|
||||
.where("username_lower <-> ? < 1", @term)
|
||||
.order(["username_lower <-> ? ASC", @term])
|
||||
.limit(@limit - users.size)
|
||||
.pluck(:id)
|
||||
.each { |id| users << id }
|
||||
end
|
||||
end
|
||||
|
||||
users.to_a
|
||||
end
|
||||
|
||||
|
@ -2580,9 +2580,6 @@ backups:
|
||||
client: true
|
||||
|
||||
search:
|
||||
user_search_similar_results:
|
||||
default: false
|
||||
hidden: true
|
||||
prioritize_exact_search_title_match:
|
||||
default: true
|
||||
hidden: true
|
||||
|
@ -0,0 +1,12 @@
|
||||
# frozen_string_literal: true
|
||||
class DropUserSearchSimilarResultsSiteSetting < ActiveRecord::Migration[7.1]
|
||||
def up
|
||||
execute <<~SQL
|
||||
DELETE FROM site_settings WHERE name = 'user_search_similar_results';
|
||||
SQL
|
||||
end
|
||||
|
||||
def down
|
||||
raise ActiveRecord::IrreversibleMigration
|
||||
end
|
||||
end
|
13
db/migrate/20240912061806_drop_trgm_indexes_on_users.rb
Normal file
13
db/migrate/20240912061806_drop_trgm_indexes_on_users.rb
Normal file
@ -0,0 +1,13 @@
|
||||
# frozen_string_literal: true
|
||||
class DropTrgmIndexesOnUsers < ActiveRecord::Migration[7.1]
|
||||
def up
|
||||
execute <<~SQL
|
||||
DROP INDEX IF EXISTS index_users_on_username_lower_trgm;
|
||||
DROP INDEX IF EXISTS index_users_on_name_trgm;
|
||||
SQL
|
||||
end
|
||||
|
||||
def down
|
||||
raise ActiveRecord::IrreversibleMigration
|
||||
end
|
||||
end
|
@ -279,44 +279,4 @@ RSpec.describe UserSearch do
|
||||
expect(results[2]).to eq("mrorange")
|
||||
end
|
||||
end
|
||||
|
||||
context "when using SiteSetting.user_search_similar_results" do
|
||||
it "should find the user even with a typo if the setting is enabled" do
|
||||
rafael = Fabricate(:user, username: "rafael", name: "Rafael Silva")
|
||||
codinghorror = Fabricate(:user, username: "codinghorror", name: "Jeff Atwood")
|
||||
pfaffman = Fabricate(:user, username: "pfaffman")
|
||||
zogstrip = Fabricate(:user, username: "zogstrip", name: "Régis Hanol")
|
||||
roman = Fabricate(:user, username: "roman", name: "Roman Rizzi")
|
||||
|
||||
SiteSetting.user_search_similar_results = false
|
||||
expect(UserSearch.new("rafel").search).to be_blank
|
||||
expect(UserSearch.new("codding").search).to be_blank
|
||||
expect(UserSearch.new("pffman").search).to be_blank
|
||||
|
||||
SiteSetting.user_search_similar_results = true
|
||||
expect(UserSearch.new("rafel").search).to include(rafael)
|
||||
expect(UserSearch.new("codding").search).to include(codinghorror)
|
||||
expect(UserSearch.new("pffman").search).to include(pfaffman)
|
||||
|
||||
SiteSetting.user_search_similar_results = false
|
||||
expect(UserSearch.new("silvia").search).to be_blank
|
||||
expect(UserSearch.new("atwod").search).to be_blank
|
||||
expect(UserSearch.new("regis").search).to be_blank
|
||||
expect(UserSearch.new("reg").search).to be_blank
|
||||
|
||||
SiteSetting.user_search_similar_results = true
|
||||
expect(UserSearch.new("silvia").search).to include(rafael)
|
||||
expect(UserSearch.new("atwod").search).to include(codinghorror)
|
||||
expect(UserSearch.new("regis").search).to include(zogstrip)
|
||||
expect(UserSearch.new("reg").search).to include(zogstrip)
|
||||
end
|
||||
|
||||
it "orders the results by similarity" do
|
||||
zogstrip = Fabricate(:user, username: "zogstrip", name: "Régis Hanol")
|
||||
roman = Fabricate(:user, username: "roman", name: "Roman Rizzi")
|
||||
SiteSetting.user_search_similar_results = true
|
||||
|
||||
expect(UserSearch.new("regis").search.first).to eq(zogstrip)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
Loading…
Reference in New Issue
Block a user