Performance improvements to comments search (#13489)
* Explicitly pluck user_id from comments table * Change comment query * Remove indexes and add partial indexes * Update schema
This commit is contained in:
parent
a0c3ef0329
commit
1b6f753f73
4 changed files with 40 additions and 26 deletions
|
|
@ -23,13 +23,6 @@ module Search
|
|||
].freeze
|
||||
private_constant :USER_ATTRIBUTES
|
||||
|
||||
ARTICLE_COMMENTABLE_QUERY = <<-SQL.freeze
|
||||
LEFT JOIN articles
|
||||
ON comments.commentable_id = articles.id
|
||||
AND comments.commentable_type = 'Article'
|
||||
SQL
|
||||
private_constant :ARTICLE_COMMENTABLE_QUERY
|
||||
|
||||
DEFAULT_PER_PAGE = 60
|
||||
private_constant :DEFAULT_PER_PAGE
|
||||
|
||||
|
|
@ -42,19 +35,6 @@ module Search
|
|||
MAX_PER_PAGE = 120 # to avoid querying too many items, we set a maximum amount for a page
|
||||
private_constant :MAX_PER_PAGE
|
||||
|
||||
# We filter comments for those that are:
|
||||
# 1. On Articles
|
||||
# 2. Not deleted
|
||||
# 3. Not hidden by commentable user (i.e. an Article author didn't hide the comment)
|
||||
# 4. Are attached to published articles
|
||||
QUERY_FILTER = <<-SQL.freeze
|
||||
comments.commentable_type = 'Article' AND
|
||||
comments.deleted = false AND
|
||||
comments.hidden_by_commentable_user = false AND
|
||||
articles.published = true
|
||||
SQL
|
||||
private_constant :QUERY_FILTER
|
||||
|
||||
def self.search_documents(
|
||||
page: 0,
|
||||
per_page: DEFAULT_PER_PAGE,
|
||||
|
|
@ -74,7 +54,14 @@ module Search
|
|||
page = page.to_i + 1
|
||||
per_page = [(per_page || DEFAULT_PER_PAGE).to_i, MAX_PER_PAGE].min
|
||||
|
||||
relation = ::Comment.joins(ARTICLE_COMMENTABLE_QUERY).where(QUERY_FILTER)
|
||||
relation = ::Comment
|
||||
.where(
|
||||
deleted: false,
|
||||
hidden_by_commentable_user: false,
|
||||
commentable_type: "Article",
|
||||
)
|
||||
.joins("join articles on articles.id = comments.commentable_id")
|
||||
.where("articles.published": true)
|
||||
|
||||
relation = relation.search_comments(term).with_pg_search_highlight if term.present?
|
||||
|
||||
|
|
@ -92,7 +79,7 @@ module Search
|
|||
# (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(:user_id)
|
||||
user_ids = results.pluck("comments.user_id")
|
||||
users = find_users(user_ids)
|
||||
|
||||
serialize(results, users)
|
||||
|
|
|
|||
15
db/migrate/20210423162805_remove_indexes_from_comments.rb
Normal file
15
db/migrate/20210423162805_remove_indexes_from_comments.rb
Normal file
|
|
@ -0,0 +1,15 @@
|
|||
class RemoveIndexesFromComments < ActiveRecord::Migration[6.1]
|
||||
disable_ddl_transaction!
|
||||
|
||||
def up
|
||||
remove_index :comments, column: :commentable_type, algorithm: :concurrently if index_exists?(:comments, :commentable_type)
|
||||
remove_index :comments, column: :deleted, algorithm: :concurrently if index_exists?(:comments, :deleted)
|
||||
remove_index :comments, column: :hidden_by_commentable_user, algorithm: :concurrently if index_exists?(:comments, :hidden_by_commentable_user)
|
||||
end
|
||||
|
||||
def down
|
||||
add_index :comments, :commentable_type, algorithm: :concurrently unless index_exists?(:comments, :commentable_type)
|
||||
add_index :comments, :deleted, algorithm: :concurrently unless index_exists?(:comments, :deleted)
|
||||
add_index :comments, :hidden_by_commentable_user, algorithm: :concurrently unless index_exists?(:comments, :hidden_by_commentable_user)
|
||||
end
|
||||
end
|
||||
|
|
@ -0,0 +1,13 @@
|
|||
class AddPartialIndexesFromComments < ActiveRecord::Migration[6.1]
|
||||
disable_ddl_transaction!
|
||||
|
||||
def up
|
||||
add_index :comments, :deleted, where: "deleted = false", algorithm: :concurrently unless index_exists?(:comments, :deleted)
|
||||
add_index :comments, :hidden_by_commentable_user,where: "hidden_by_commentable_user = false", algorithm: :concurrently unless index_exists?(:comments, :hidden_by_commentable_user)
|
||||
end
|
||||
|
||||
def down
|
||||
remove_index :comments, column: :deleted, algorithm: :concurrently if index_exists?(:comments, :deleted)
|
||||
remove_index :comments, column: :hidden_by_commentable_user, algorithm: :concurrently if index_exists?(:comments, :hidden_by_commentable_user)
|
||||
end
|
||||
end
|
||||
|
|
@ -10,7 +10,7 @@
|
|||
#
|
||||
# It's strongly recommended that you check this file into your version control system.
|
||||
|
||||
ActiveRecord::Schema.define(version: 2021_04_22_171642) do
|
||||
ActiveRecord::Schema.define(version: 2021_04_23_162847) do
|
||||
|
||||
# These are extensions that must be enabled in order to support this database
|
||||
enable_extension "citext"
|
||||
|
|
@ -392,10 +392,9 @@ ActiveRecord::Schema.define(version: 2021_04_22_171642) do
|
|||
t.index ["ancestry"], name: "index_comments_on_ancestry"
|
||||
t.index ["ancestry"], name: "index_comments_on_ancestry_trgm", opclass: :gin_trgm_ops, using: :gin
|
||||
t.index ["commentable_id", "commentable_type"], name: "index_comments_on_commentable_id_and_commentable_type"
|
||||
t.index ["commentable_type"], name: "index_comments_on_commentable_type"
|
||||
t.index ["created_at"], name: "index_comments_on_created_at"
|
||||
t.index ["deleted"], name: "index_comments_on_deleted"
|
||||
t.index ["hidden_by_commentable_user"], name: "index_comments_on_hidden_by_commentable_user"
|
||||
t.index ["deleted"], name: "index_comments_on_deleted", where: "(deleted = false)"
|
||||
t.index ["hidden_by_commentable_user"], name: "index_comments_on_hidden_by_commentable_user", where: "(hidden_by_commentable_user = false)"
|
||||
t.index ["score"], name: "index_comments_on_score"
|
||||
t.index ["user_id"], name: "index_comments_on_user_id"
|
||||
end
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue