Skip to content

Commit

Permalink
Replace incorrect use of distinct with group (mastodon#14675)
Browse files Browse the repository at this point in the history
Some uses of ActiveRecord::QueryMethods#distinct pass field names but they
are incorrect for the current version of Rails.

ActiveRecord::QueryMethods#group provides the expected behavior and
benefits performance. See commit 6da24aa.
  • Loading branch information
akihikodaki authored and thenameisnigel-old committed Sep 6, 2020
1 parent 17edcf1 commit 726e132
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 14 deletions.
12 changes: 1 addition & 11 deletions app/controllers/api/v1/accounts/statuses_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,7 @@ def permitted_account_statuses
end

def only_media_scope
Status.where(id: account_media_status_ids)
end

def account_media_status_ids
# `SELECT DISTINCT id, updated_at` is too slow, so pluck ids at first, and then select id, updated_at with ids.
# Also, Avoid getting slow by not narrowing down by `statuses.account_id`.
# When narrowing down by `statuses.account_id`, `index_statuses_20180106` will be used
# and the table will be joined by `Merge Semi Join`, so the query will be slow.
@account.statuses.joins(:media_attachments).merge(@account.media_attachments).permitted_for(@account, current_account)
.paginate_by_max_id(limit_param(DEFAULT_STATUSES_LIMIT), params[:max_id], params[:since_id])
.reorder(id: :desc).distinct(:id).pluck(:id)
Status.joins(:media_attachments).merge(@account.media_attachments.reorder(nil)).group(:id)
end

def pinned_scope
Expand Down
4 changes: 1 addition & 3 deletions app/controllers/api/v1/timelines/public_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@ def public_statuses
)

if truthy_param?(:only_media)
# `SELECT DISTINCT id, updated_at` is too slow, so pluck ids at first, and then select id, updated_at with ids.
status_ids = statuses.joins(:media_attachments).distinct(:id).pluck(:id)
statuses.where(id: status_ids)
statuses.joins(:media_attachments).group(:id)
else
statuses
end
Expand Down

0 comments on commit 726e132

Please sign in to comment.