User search: Remove trigram and refactor the module
- Remove trigram as it tends to rank garbage results highly, resulting in it prioritized above fts, which gives actually decent results. ACKed by kaniini and lain on irc. - Remove a test for handling misspelled requests, since we no longer have trigram - Remove a test for searching users with `nil` display names, because it is unrealistic, we don't accept usernames that are not >1 char strings - Make rank boosting for followers/followees sane again, previous values resulted in garbage matches getting on top just because the users are followers/followees
This commit is contained in:
rinpatch
parent
3b1bc079af
commit
c10ce113d4
2 changed files with 50 additions and 119 deletions
|
|
@ -4,7 +4,6 @@
|
|||
|
||||
defmodule Pleroma.User.Search do
|
||||
alias Pleroma.Pagination
|
||||
alias Pleroma.Repo
|
||||
alias Pleroma.User
|
||||
import Ecto.Query
|
||||
|
||||
|
|
@ -23,18 +22,10 @@ def search(query_string, opts \\ []) do
|
|||
|
||||
maybe_resolve(resolve, for_user, query_string)
|
||||
|
||||
{:ok, results} =
|
||||
Repo.transaction(fn ->
|
||||
Ecto.Adapters.SQL.query(
|
||||
Repo,
|
||||
"select set_limit(#{@similarity_threshold})",
|
||||
[]
|
||||
)
|
||||
|
||||
query_string
|
||||
|> search_query(for_user, following)
|
||||
|> Pagination.fetch_paginated(%{"offset" => offset, "limit" => result_limit}, :offset)
|
||||
end)
|
||||
results =
|
||||
query_string
|
||||
|> search_query(for_user, following)
|
||||
|> Pagination.fetch_paginated(%{"offset" => offset, "limit" => result_limit}, :offset)
|
||||
|
||||
results
|
||||
end
|
||||
|
|
@ -56,15 +47,52 @@ defp search_query(query_string, for_user, following) do
|
|||
|> base_query(following)
|
||||
|> filter_blocked_user(for_user)
|
||||
|> filter_blocked_domains(for_user)
|
||||
|> search_subqueries(query_string)
|
||||
|> union_subqueries
|
||||
|> distinct_query()
|
||||
|> boost_search_rank_query(for_user)
|
||||
|> fts_subquery(query_string)
|
||||
|> subquery()
|
||||
|> where([u], u.search_rank > @similarity_threshold)
|
||||
|> boost_search_rank(for_user)
|
||||
|> order_by(desc: :search_rank)
|
||||
|> maybe_restrict_local(for_user)
|
||||
end
|
||||
|
||||
@nickname_regex ~r/^[a-zA-Z0-9.!#$%&'*+\/=?^_`{|}~\-@]+$/
|
||||
defp fts_subquery(query, query_string) do
|
||||
{nickname_weight, name_weight} =
|
||||
if String.match?(query_string, @nickname_regex) do
|
||||
{"A", "B"}
|
||||
else
|
||||
{"B", "A"}
|
||||
end
|
||||
|
||||
query_string = to_tsquery(query_string)
|
||||
|
||||
from(
|
||||
u in query,
|
||||
select_merge: %{
|
||||
search_rank:
|
||||
fragment(
|
||||
"""
|
||||
ts_rank_cd((setweight(to_tsvector('simple', ?), ?) || setweight(to_tsvector('simple', ?), ?)), to_tsquery('simple', ?))
|
||||
""",
|
||||
u.name,
|
||||
^name_weight,
|
||||
u.nickname,
|
||||
^nickname_weight,
|
||||
^query_string
|
||||
)
|
||||
}
|
||||
)
|
||||
end
|
||||
|
||||
defp to_tsquery(query_string) do
|
||||
String.trim_trailing(query_string, "@" <> local_domain())
|
||||
|> String.replace(~r/[!-\/|@|[-`|{-~|:-?]+/, " ")
|
||||
|> String.trim()
|
||||
|> String.split()
|
||||
|> Enum.map(&(&1 <> ":*"))
|
||||
|> Enum.join(" | ")
|
||||
end
|
||||
|
||||
defp base_query(_user, false), do: User
|
||||
defp base_query(user, true), do: User.get_followers_query(user)
|
||||
|
||||
|
|
@ -87,21 +115,6 @@ defp filter_blocked_domains(query, %User{info: %{domain_blocks: domain_blocks}})
|
|||
|
||||
defp filter_blocked_domains(query, _), do: query
|
||||
|
||||
defp union_subqueries({fts_subquery, trigram_subquery}) do
|
||||
from(s in trigram_subquery, union_all: ^fts_subquery)
|
||||
end
|
||||
|
||||
defp search_subqueries(base_query, query_string) do
|
||||
{
|
||||
fts_search_subquery(base_query, query_string),
|
||||
trigram_search_subquery(base_query, query_string)
|
||||
}
|
||||
end
|
||||
|
||||
defp distinct_query(q) do
|
||||
from(s in subquery(q), order_by: s.search_type, distinct: s.id)
|
||||
end
|
||||
|
||||
defp maybe_resolve(true, user, query) do
|
||||
case {limit(), user} do
|
||||
{:all, _} -> :noop
|
||||
|
|
@ -126,9 +139,9 @@ defp limit, do: Pleroma.Config.get([:instance, :limit_to_local_content], :unauth
|
|||
|
||||
defp restrict_local(q), do: where(q, [u], u.local == true)
|
||||
|
||||
defp boost_search_rank_query(query, nil), do: query
|
||||
defp local_domain, do: Pleroma.Config.get([Pleroma.Web.Endpoint, :url, :host])
|
||||
|
||||
defp boost_search_rank_query(query, for_user) do
|
||||
defp boost_search_rank(query, %User{} = for_user) do
|
||||
friends_ids = User.get_friends_ids(for_user)
|
||||
followers_ids = User.get_followers_ids(for_user)
|
||||
|
||||
|
|
@ -137,8 +150,8 @@ defp boost_search_rank_query(query, for_user) do
|
|||
search_rank:
|
||||
fragment(
|
||||
"""
|
||||
CASE WHEN (?) THEN 0.5 + (?) * 1.3
|
||||
WHEN (?) THEN 0.5 + (?) * 1.2
|
||||
CASE WHEN (?) THEN (?) * 1.5
|
||||
WHEN (?) THEN (?) * 1.3
|
||||
WHEN (?) THEN (?) * 1.1
|
||||
ELSE (?) END
|
||||
""",
|
||||
|
|
@ -154,70 +167,5 @@ defp boost_search_rank_query(query, for_user) do
|
|||
)
|
||||
end
|
||||
|
||||
@spec fts_search_subquery(User.t() | Ecto.Query.t(), String.t()) :: Ecto.Query.t()
|
||||
defp fts_search_subquery(query, term) do
|
||||
processed_query =
|
||||
String.trim_trailing(term, "@" <> local_domain())
|
||||
|> String.replace(~r/[!-\/|@|[-`|{-~|:-?]+/, " ")
|
||||
|> String.trim()
|
||||
|> String.split()
|
||||
|> Enum.map(&(&1 <> ":*"))
|
||||
|> Enum.join(" | ")
|
||||
|
||||
from(
|
||||
u in query,
|
||||
select_merge: %{
|
||||
search_type: ^0,
|
||||
search_rank:
|
||||
fragment(
|
||||
"""
|
||||
ts_rank_cd(
|
||||
setweight(to_tsvector('simple', regexp_replace(?, '\\W', ' ', 'g')), 'A') ||
|
||||
setweight(to_tsvector('simple', regexp_replace(coalesce(?, ''), '\\W', ' ', 'g')), 'B'),
|
||||
to_tsquery('simple', ?),
|
||||
32
|
||||
)
|
||||
""",
|
||||
u.nickname,
|
||||
u.name,
|
||||
^processed_query
|
||||
)
|
||||
},
|
||||
where:
|
||||
fragment(
|
||||
"""
|
||||
(setweight(to_tsvector('simple', regexp_replace(?, '\\W', ' ', 'g')), 'A') ||
|
||||
setweight(to_tsvector('simple', regexp_replace(coalesce(?, ''), '\\W', ' ', 'g')), 'B')) @@ to_tsquery('simple', ?)
|
||||
""",
|
||||
u.nickname,
|
||||
u.name,
|
||||
^processed_query
|
||||
)
|
||||
)
|
||||
|> User.restrict_deactivated()
|
||||
end
|
||||
|
||||
@spec trigram_search_subquery(User.t() | Ecto.Query.t(), String.t()) :: Ecto.Query.t()
|
||||
defp trigram_search_subquery(query, term) do
|
||||
term = String.trim_trailing(term, "@" <> local_domain())
|
||||
|
||||
from(
|
||||
u in query,
|
||||
select_merge: %{
|
||||
# ^1 gives 'Postgrex expected a binary, got 1' for some weird reason
|
||||
search_type: fragment("?", 1),
|
||||
search_rank:
|
||||
fragment(
|
||||
"similarity(?, trim(? || ' ' || coalesce(?, '')))",
|
||||
^term,
|
||||
u.nickname,
|
||||
u.name
|
||||
)
|
||||
},
|
||||
where: fragment("trim(? || ' ' || coalesce(?, '')) % ?", u.nickname, u.name, ^term)
|
||||
)
|
||||
|> User.restrict_deactivated()
|
||||
end
|
||||
|
||||
defp local_domain, do: Pleroma.Config.get([Pleroma.Web.Endpoint, :url, :host])
|
||||
defp boost_search_rank(query, _for_user), do: query
|
||||
end
|
||||
|
|
|
|||
|
|
@ -74,12 +74,6 @@ test "finds users, ranking by similarity" do
|
|||
assert [u4.id, u3.id, u1.id] == Enum.map(User.search("lain@ple", for_user: u1), & &1.id)
|
||||
end
|
||||
|
||||
test "finds users, handling misspelled requests" do
|
||||
u1 = insert(:user, %{name: "lain"})
|
||||
|
||||
assert [u1.id] == Enum.map(User.search("laiin"), & &1.id)
|
||||
end
|
||||
|
||||
test "finds users, boosting ranks of friends and followers" do
|
||||
u1 = insert(:user)
|
||||
u2 = insert(:user, %{name: "Doe"})
|
||||
|
|
@ -163,17 +157,6 @@ test "find all users for unauthenticated users when `limit_to_local_content` is
|
|||
Pleroma.Config.put([:instance, :limit_to_local_content], :unauthenticated)
|
||||
end
|
||||
|
||||
test "finds a user whose name is nil" do
|
||||
_user = insert(:user, %{name: "notamatch", nickname: "testuser@pleroma.amplifie.red"})
|
||||
user_two = insert(:user, %{name: nil, nickname: "lain@pleroma.soykaf.com"})
|
||||
|
||||
assert user_two ==
|
||||
User.search("lain@pleroma.soykaf.com")
|
||||
|> List.first()
|
||||
|> Map.put(:search_rank, nil)
|
||||
|> Map.put(:search_type, nil)
|
||||
end
|
||||
|
||||
test "does not yield false-positive matches" do
|
||||
insert(:user, %{name: "John Doe"})
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue