PERF: Optimize CategoryList#relevant_topics_query SQL query (stable backport) (#36571)

nullchristo · tgxworld · web-flow · commit 52d544e7e440 · 2025-12-09T15:04:13.000-06:00
Topics marked as category featured topics should always have `Topic#category_id` set so the `topics.category_id is null` predicate is not necessary. Profiling the query also reviewes that the `topics.category_id is null` predicate is causing the PG planner to do a seq scan against the topics table on large sites. Backports #36476 Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
diff --git a/app/models/category_list.rb b/app/models/category_list.rb
@@ -76,7 +76,11 @@ def self.order_categories(categories)
   def relevant_topics_query
     @all_topics =
       Topic
-        .secured(@guardian)
+        .secured(
+          @guardian,
+          include_uncategorized: false,
+        ) # perf optimization since category featured topics can't have `category_id` set to null
+        .listable_topics
         .joins(
           "INNER JOIN category_featured_topics ON topics.id = category_featured_topics.topic_id",
         )
diff --git a/app/models/topic.rb b/app/models/topic.rb
@@ -366,7 +366,7 @@ def recover!(recovered_by = nil)
   scope :exclude_scheduled_bump_topics, -> { where.not(id: TopicTimer.scheduled_bump_topics) }
 
   scope :secured,
-        lambda { |guardian = nil|
+        lambda { |guardian = nil, include_uncategorized: true|
           ids = guardian.secure_category_ids if guardian
 
           # Query conditions
@@ -377,8 +377,10 @@ def recover!(recovered_by = nil)
               ["NOT read_restricted"]
             end
 
+          uncategorized_condition = "topics.category_id IS NULL OR" if include_uncategorized
+
           where(
-            "topics.category_id IS NULL OR topics.category_id IN (SELECT id FROM categories WHERE #{condition[0]})",
+            "#{uncategorized_condition} topics.category_id IN (SELECT id FROM categories WHERE #{condition[0]})",
             condition[1],
           )
         }
diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb
@@ -2543,6 +2543,19 @@ def set_state!(group, user, state)
         topic,
       )
     end
+
+    it "excludes uncategorized topics when `include_uncategorized` kwarg is false" do
+      uncategorized_topic =
+        Fabricate(:topic, category_id: nil, archetype: Archetype.private_message)
+
+      categorized_topic = Fabricate(:topic, category: Fabricate(:category))
+
+      expect(uncategorized_topic.reload.category_id).to eq(nil)
+
+      result = Topic.secured(Guardian.new(user), include_uncategorized: false)
+
+      expect(result.pluck(:id)).to contain_exactly(categorized_topic.id)
+    end
   end
 
   describe "all_allowed_users" do
