Preload users instead of querying separately when searching for comments (#13490)

* Preload users instead of querying separately

* Add conditional to comment.user
This commit is contained in:
Alex 2021-04-23 16:30:06 -04:00 committed by GitHub
parent 2f703e97f3
commit f233be805b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 15 additions and 38 deletions

View file

@ -3,8 +3,8 @@ module Search
class PostgresCommentSerializer < ApplicationSerializer
attribute :id, &:search_id
attributes :path do |comment, params|
user = params[:users][comment.user_id]
attributes :path do |comment|
user = comment.user
if user
"/#{user.username}/comment/#{comment.id_code_generated}"
@ -19,7 +19,7 @@ module Search
attribute :class_name, -> { "Comment" }
attribute :highlight do |comment, _params|
attribute :highlight do |comment|
{
body_text: [comment.pg_search_highlight]
}
@ -40,9 +40,14 @@ module Search
# NOTE: not using the `NestedUserSerializer` to avoid hitting Redis to
# fetch the cached value
attribute :user do |comment, params|
user = params[:users][comment.user_id]
user.slice(:name, :profile_image_90, :username).symbolize_keys
attribute :user do |comment|
user = comment.user
if user
user.slice(:name, :profile_image_90, :username).symbolize_keys
else
{}
end
end
end
end

View file

@ -15,14 +15,6 @@ module Search
].freeze
private_constant :ATTRIBUTES
USER_ATTRIBUTES = %i[
id
name
profile_image
username
].freeze
private_constant :USER_ATTRIBUTES
DEFAULT_PER_PAGE = 60
private_constant :DEFAULT_PER_PAGE
@ -55,6 +47,7 @@ module Search
per_page = [(per_page || DEFAULT_PER_PAGE).to_i, MAX_PER_PAGE].min
relation = ::Comment
.includes(:user)
.where(
deleted: false,
hidden_by_commentable_user: false,
@ -69,33 +62,12 @@ module Search
results = relation.page(page).per(per_page)
# NOTE: [@rhymes/atsmith813] an earlier version used `.includes(:user)`
# to preload users, unfortunately it's not possible in Rails to specify
# which fields of the included relation's table to select ahead of time.
# The `users` table is massive (115 columns on March 2021) and thus we
# shouldn't load it all in memory just to select a few fields.
# For these reasons I decided to avoid preloading altogether and issue
# an additional SQL query to load User objects
# (see https://github.com/forem/forem/pull/4744#discussion_r345698674
# and https://github.com/rails/rails/issues/15185#issuecomment-351868335
# for additional context)
user_ids = results.pluck("comments.user_id")
users = find_users(user_ids)
serialize(results, users)
serialize(results)
end
def self.find_users(user_ids)
::User
.where(id: user_ids)
.select(*USER_ATTRIBUTES)
.index_by(&:id)
end
private_class_method :find_users
def self.serialize(results, users)
def self.serialize(results)
Search::PostgresCommentSerializer
.new(results, params: { users: users }, is_collection: true)
.new(results, is_collection: true)
.serializable_hash[:data]
.pluck(:attributes)
end