FIX: Move emotion /filter logic into a CTE to keep cardinality sane (#…

…915)
discourse · Nov 14, 2024 · 4fb686a · 4fb686a
1 parent 5026ab5
commit 4fb686a
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 29 deletions.
diff --git a/lib/sentiment/emotion_filter_order.rb b/lib/sentiment/emotion_filter_order.rb
@@ -39,34 +39,43 @@ def self.register!(plugin)
           filter_order_emotion = ->(scope, order_direction) do
             emotion_clause = <<~SQL
               SUM(
-                CASE 
-                  WHEN (classification_results.classification::jsonb->'#{emotion}')::float > 0.1 
-                  THEN 1 
-                  ELSE 0 
+                CASE
+                  WHEN (classification_results.classification::jsonb->'#{emotion}')::float > 0.1
+                  THEN 1
+                  ELSE 0
                 END
                )::float / COUNT(posts.id)
             SQL
+
+            # TODO: This is slow, we will need to materialize this in the future
+            with_clause = <<~SQL
+                SELECT
+                    topics.id,
+                    #{emotion_clause} AS emotion_#{emotion}
+                FROM
+                    topics
+                INNER JOIN
+                    posts ON posts.topic_id = topics.id
+                INNER JOIN
+                    classification_results ON
+                    classification_results.target_id = posts.id AND
+                    classification_results.target_type = 'Post' AND
+                    classification_results.model_used = 'SamLowe/roberta-base-go_emotions'
+                WHERE
+                    topics.archetype = 'regular'
+                    AND topics.deleted_at IS NULL
+                    AND posts.deleted_at IS NULL
+                    AND posts.post_type = 1
+                GROUP BY
+                    1
+                HAVING
+                    #{emotion_clause} > 0.05
+            SQL
+
             scope
-              .joins(:posts)
-              .joins(<<~SQL)
-                INNER JOIN classification_results 
-                  ON classification_results.target_id = posts.id 
-                  AND classification_results.target_type = 'Post' 
-                  AND classification_results.model_used = 'SamLowe/roberta-base-go_emotions'
-              SQL
-              .where(<<~SQL)
-                topics.archetype = 'regular' 
-                AND topics.deleted_at IS NULL 
-                AND posts.deleted_at IS NULL 
-                AND posts.post_type = 1
-              SQL
-              .select(<<~SQL)
-                topics.*, 
-                #{emotion_clause} AS emotion_#{emotion} 
-              SQL
-              .group("1")
-              .having("#{emotion_clause} > 0.05")
-              .order("#{emotion_clause} #{order_direction}")
+              .with(topic_emotion: Arel.sql(with_clause))
+              .joins("INNER JOIN topic_emotion ON topic_emotion.id = topics.id")
+              .order("topic_emotion.emotion_#{emotion} #{order_direction}")
           end
           plugin.add_filter_custom_filter("order:emotion_#{emotion}", &filter_order_emotion)
         end

diff --git a/spec/lib/modules/sentiment/emotion_filter_order_spec.rb b/spec/lib/modules/sentiment/emotion_filter_order_spec.rb
@@ -181,14 +181,11 @@
         .first
     result = filter.call(scope, order_direction)
 
-    expect(result.to_sql).to include("INNER JOIN classification_results")
+    expect(result.to_sql).to include("classification_results")
     expect(result.to_sql).to include(
       "classification_results.model_used = 'SamLowe/roberta-base-go_emotions'",
     )
-    expect(result.to_sql).to include("topics.archetype = 'regular'")
-    expect(result.to_sql).to include("ORDER BY")
-    expect(result.to_sql).to include("->'#{emotion}'")
-    expect(result.to_sql).to include("desc")
+    expect(result.to_sql).to include("ORDER BY topic_emotion.emotion_joy desc")
   end
 
   it "sorts emotion in ascending order" do