Skip to content

Commit

Permalink
PERF: refactor user search so works more efficiently
Browse files Browse the repository at this point in the history
Stop scanning entire user table
  • Loading branch information
SamSaffron committed May 14, 2015
1 parent 1eeed5f commit e074651
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 22 deletions.
83 changes: 63 additions & 20 deletions app/models/user_search.rb
Expand Up @@ -10,10 +10,29 @@ def initialize(term, opts={})
@limit = opts[:limit] || 20
end

def search
users = User.order(User.sql_fragment("CASE WHEN username_lower = ? THEN 0 ELSE 1 END ASC", @term.downcase))
def scoped_users
users = User.where("active")

unless @searching_user && @searching_user.staff?
users = users.not_suspended
end

# Only show users who have access to private topic
if @topic_id && @topic_allowed_users == "true"
topic = Topic.find_by(id: @topic_id)

if topic.category && topic.category.read_restricted
users = users.includes(:secure_categories)
.where("users.admin = TRUE OR categories.id = ?", topic.category.id)
.references(:categories)
end
end

users = users.where(active: true)
users.limit(@limit)
end

def filtered_by_term_users
users = scoped_users

if @term.present?
if SiteSetting.enable_names?
Expand All @@ -28,29 +47,53 @@ def search
end
end

if @topic_id
users = users.joins(User.sql_fragment("LEFT JOIN (SELECT DISTINCT p.user_id FROM POSTS p WHERE topic_id = ?) s ON s.user_id = users.id", @topic_id))
.order("CASE WHEN s.user_id IS NULL THEN 0 ELSE 1 END DESC")
end
users
end

def search_ids
users = Set.new

# 1. exact username matches
if @term.present?
scoped_users.where(username_lower: @term.downcase)
.pluck(:id)
.each{|id| users << id}

unless @searching_user && @searching_user.staff?
users = users.not_suspended
end

# Only show users who have access to private topic
if @topic_id && @topic_allowed_users == "true"
allowed_user_ids = []
topic = Topic.find_by(id: @topic_id)
return users.to_a if users.length == @limit

if topic.category && topic.category.read_restricted
users = users.includes(:secure_categories)
.where("users.admin = TRUE OR categories.id = ?", topic.category.id)
.references(:categories)
end
# 2. in topic
if @topic_id
filtered_by_term_users.where('users.id in (SELECT p.user_id FROM posts p WHERE topic_id = ?)', @topic_id)
.order('last_seen_at DESC')
.limit(@limit - users.length)
.pluck(:id)
.each{|id| users << id}
end

users.order("CASE WHEN last_seen_at IS NULL THEN 0 ELSE 1 END DESC, last_seen_at DESC, username ASC")
.limit(@limit)
return users.to_a if users.length == @limit

# 3. global matches
filtered_by_term_users.order('last_seen_at DESC')
.limit(@limit - users.length)
.pluck(:id)
.each{|id| users << id}

users.to_a

end

def search

ids = search_ids
return User.where("0=1") if ids.empty?

User.joins("JOIN (SELECT unnest uid, row_number() OVER () AS rn
FROM unnest('{#{ids.join(",")}}'::int[])
) x on uid = users.id")
.order("rn")

end

end
4 changes: 2 additions & 2 deletions spec/models/user_search_spec.rb
Expand Up @@ -38,7 +38,7 @@ def search_for(*args)
# normal search
results = search_for(user1.name.split(" ").first)
expect(results.size).to eq(1)
expect(results.first).to eq(user1)
expect(results.first.username).to eq(user1.username)

# lower case
results = search_for(user1.name.split(" ").first.downcase)
Expand Down Expand Up @@ -106,7 +106,7 @@ def search_for(*args)

# find an exact match first
results = search_for("mrB")
expect(results.first).to eq(user1)
expect(results.first.username).to eq(user1.username)

# don't return inactive users
results = search_for("Ghost")
Expand Down

0 comments on commit e074651

Please sign in to comment.