From def0cf2437df3b6ac6b3fdf7e843d76f514b92e5 Mon Sep 17 00:00:00 2001 From: rhymes Date: Mon, 25 Jan 2021 17:34:10 +0100 Subject: [PATCH] Optimize column selection for articles belonging to "any" given tag (#12420) When `acts-as-taggable-on`'s `.tagged_with()` is used with `any: true`, the gem will use `SELECT *` regardless of any previous (or following) requests of selecting a limited amount of columns. Given that the `articles` table has [73 columns](https://dev.to/admin/blazer/queries/314-number-of-columns-in-all-tables) that will amount to wasted RAM memory for columns we don't need. By "unscoping" any previous `select()` we can optimize used memory. Before: ```ruby [24] pry(main)> Article.tagged_with([:ruby], any: true).select(:id, :name).to_sql => "SELECT \"articles\".*, \"articles\".\"id\", \"name\" FROM \"articles\" WHERE EXISTS (SELECT * FROM \"taggings\" WHERE \"taggings\".\"taggable_id\" = \"articles\".\"id\" AND \"taggings\".\"taggable_type\" = 'Article' AND \"taggings\".\"tag_id\" IN (SELECT \"tags\".\"id\" FROM \"tags\" WHERE (\"tags\".\"name\" LIKE 'ruby' ESCAPE '!')))" ``` Note, how the SQL query is `articles.*, articles.column_a` After: [25] pry(main)> Article.tagged_with([:ruby], any: true).unscope(:select).select(:id, :name).to_sql => "SELECT \"articles\".\"id\", \"name\" FROM \"articles\" WHERE EXISTS (SELECT * FROM \"taggings\" WHERE \"taggings\".\"taggable_id\" = \"articles\".\"id\" AND \"taggings\".\"taggable_type\" = 'Article' AND \"taggings\".\"tag_id\" IN (SELECT \"tags\".\"id\" FROM \"tags\" WHERE (\"tags\".\"name\" LIKE 'ruby' ESCAPE '!')))" ``` `articles.*` is gone :-) - https://github.com/mbleigh/acts-as-taggable-on/issues/936 - https://github.com/mbleigh/acts-as-taggable-on/blob/47da5036dea61cb971bfaf72de5fa93c85255307/lib/acts_as_taggable_on/taggable/tagged_with_query/any_tags_query.rb#L2-L8 --- app/controllers/admin/articles_controller.rb | 2 +- app/models/user.rb | 3 ++- app/services/article_api_index_service.rb | 2 +- app/services/articles/feeds/large_forem_experimental.rb | 8 ++++++-- app/services/articles/get_user_stickies.rb | 2 +- app/services/articles/suggest_stickies.rb | 4 ++-- 6 files changed, 13 insertions(+), 8 deletions(-) diff --git a/app/controllers/admin/articles_controller.rb b/app/controllers/admin/articles_controller.rb index 5e454396e..7cbea134a 100644 --- a/app/controllers/admin/articles_controller.rb +++ b/app/controllers/admin/articles_controller.rb @@ -73,7 +73,7 @@ module Admin def articles_satellite Article.published.where(last_buffered: nil) .includes(:user, :buffer_updates) - .tagged_with(Tag.bufferized_tags, any: true) + .tagged_with(Tag.bufferized_tags, any: true).unscope(:select) .limited_columns_internal_select .order(hotness_score: :desc) .page(params[:page]) diff --git a/app/models/user.rb b/app/models/user.rb index 518bffa99..0d9d1ebec 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -304,7 +304,8 @@ class User < ApplicationRecord end def followed_articles - Article.tagged_with(cached_followed_tag_names, any: true) + Article + .tagged_with(cached_followed_tag_names, any: true).unscope(:select) .union(Article.where(user_id: cached_following_users_ids)) .where(language: preferred_languages_array, published: true) end diff --git a/app/services/article_api_index_service.rb b/app/services/article_api_index_service.rb index f79810950..0a07d5b31 100644 --- a/app/services/article_api_index_service.rb +++ b/app/services/article_api_index_service.rb @@ -77,7 +77,7 @@ class ArticleApiIndexService def tagged_articles articles = Article.published.includes(:user, :organization) - articles = articles.tagged_with(tags, any: true) if tags + articles = articles.tagged_with(tags, any: true).unscope(:select) if tags articles = articles.tagged_with(tags_exclude, exclude: true) if tags_exclude articles diff --git a/app/services/articles/feeds/large_forem_experimental.rb b/app/services/articles/feeds/large_forem_experimental.rb index fd9d2026e..a6eb54266 100644 --- a/app/services/articles/feeds/large_forem_experimental.rb +++ b/app/services/articles/feeds/large_forem_experimental.rb @@ -158,12 +158,14 @@ module Articles .order(hotness_score: :desc) when "only_followed_tags" # equivalent to base but only on tags user follows (if user follows enough) followed_tags = @user.cached_followed_tag_names - articles = Article.published.limited_column_select.includes(top_comments: :user) + articles = Article.published.includes(top_comments: :user) .page(@page).per(@number_of_articles) .where("score >= ? OR featured = ?", SiteConfig.home_feed_minimum_score, true) .order(hotness_score: :desc) # We only want to limit the posts to tagged_with if the participant follows enough tags. articles = articles.tagged_with(followed_tags, any: true) if followed_tags.size > 4 + + articles = articles.unscope(:select).limited_column_select when "top_articles_since_last_pageview_3_days_max" # Top articles since last page view (max 3 days) start_time = [(@user.page_views.last&.created_at || 3.days.ago) - 12.hours, 3.days.ago].max articles = Article.published.limited_column_select.includes(top_comments: :user) @@ -179,12 +181,14 @@ module Articles when "combination_only_tags_followed_and_top_max_7_days" # Top articles since last page view (max 7 days) start_time = [(@user.page_views.last&.created_at || 7.days.ago) - 12.hours, 7.days.ago].max followed_tags = @user.cached_followed_tag_names - articles = Article.published.limited_column_select.includes(top_comments: :user) + articles = Article.published.includes(top_comments: :user) .where("published_at > ?", start_time) .page(@page).per(@number_of_articles) .order(score: :desc) # We only want to limit the posts to tagged_with if the participant follows enough tags. articles = articles.tagged_with(followed_tags, any: true) if followed_tags.size > 4 + + articles = articles.unscope(:select).limited_column_select else # "base" articles = Article.published.limited_column_select.includes(top_comments: :user) .page(@page).per(@number_of_articles) diff --git a/app/services/articles/get_user_stickies.rb b/app/services/articles/get_user_stickies.rb index 7e09d2dec..86fadefc1 100644 --- a/app/services/articles/get_user_stickies.rb +++ b/app/services/articles/get_user_stickies.rb @@ -6,8 +6,8 @@ module Articles author .articles .published + .tagged_with(article_tags, any: true).unscope(:select) .limited_column_select - .tagged_with(article_tags, any: true) .where.not(id: article.id) .order(published_at: :desc) .limit(3) diff --git a/app/services/articles/suggest_stickies.rb b/app/services/articles/suggest_stickies.rb index 1690a2082..3e9e7544e 100644 --- a/app/services/articles/suggest_stickies.rb +++ b/app/services/articles/suggest_stickies.rb @@ -25,7 +25,7 @@ module Articles Article .published - .tagged_with(article_tags, any: true) + .tagged_with(article_tags, any: true).unscope(:select) .limited_column_select .where("public_reactions_count > ? OR comments_count > ?", reaction_count_num, comment_count_num) .where.not(id: article.id) @@ -40,7 +40,7 @@ module Articles Article .published - .tagged_with(SUGGESTION_TAGS, any: true) + .tagged_with(SUGGESTION_TAGS, any: true).unscope(:select) .limited_column_select .where("comments_count > ?", comment_count_num) .where.not(id: article.id)