PERF: Don't pluck all the columns just to retrieve a single value.

app/serializers/topic_view_serializer.rb

@@ -186,10 +186,8 @@ class TopicViewSerializer < ApplicationSerializer
   end
 
   def last_read_post_id
-    return nil unless object.filtered_post_stream && last_read_post_number
-    object.filtered_post_stream.each do |ps|
-      return ps[0] if ps[1] === last_read_post_number
-    end
+    return nil unless last_read_post_number
+    object.last_read_post_id(last_read_post_number)
   end
   alias_method :include_last_read_post_id?, :has_topic_user?
 

lib/timeline_lookup.rb

@@ -1,6 +1,6 @@
 module TimelineLookup
 
-  # Given an array of tuples (id, post_number, days_ago), return at most `max_values` worth of a
+  # Given an array of tuples containing (id, days_ago), return at most `max_values` worth of a
   # lookup table to help the front end timeline display dates associated with posts
   def self.build(tuples, max_values = 300)
     result = []
@@ -9,9 +9,10 @@ module TimelineLookup
 
     last_days_ago = -1
     tuples.each_with_index do |t, idx|
+      return result unless t.is_a?(Array)
       next unless (idx % every) === 0
 
-      days_ago = t[2]
+      days_ago = t[1]
 
       if (days_ago != last_days_ago)
         result << [idx + 1, days_ago]

lib/topic_view.rb

@@ -374,14 +374,14 @@ class TopicView
     @filtered_posts.by_newest.with_user.first(25)
   end
 
-  # Returns an array of [id, post_number, days_ago] tuples.
+  # Returns an array of [id, days_ago] tuples.
   # `days_ago` is there for the timeline calculations.
   def filtered_post_stream
     @filtered_post_stream ||= begin
       posts = @filtered_posts
         .order(:sort_order)
 
-      columns = [:id, :post_number]
+      columns = [:id]
 
       if !is_mega_topic?
         columns << 'EXTRACT(DAYS FROM CURRENT_TIMESTAMP - created_at)::INT AS days_ago'
@@ -392,7 +392,13 @@ class TopicView
   end
 
   def filtered_post_ids
-    @filtered_post_ids ||= filtered_post_stream.map { |tuple| tuple[0] }
+    @filtered_post_ids ||= filtered_post_stream.map do |tuple|
+      if is_mega_topic?
+        tuple
+      else
+        tuple[0]
+      end
+    end
   end
 
   def unfiltered_post_ids
@@ -406,6 +412,10 @@ class TopicView
       end
   end
 
+  def last_read_post_id(post_number)
+    @filtered_posts.where(post_number: post_number).pluck(:id).first
+  end
+
   protected
 
   def read_posts_set
@@ -571,6 +581,6 @@ class TopicView
   MEGA_TOPIC_POSTS_COUNT = 10000
 
   def is_mega_topic?
-    @topic.posts_count >= MEGA_TOPIC_POSTS_COUNT
+    @is_mega_topic ||= (@topic.posts_count >= MEGA_TOPIC_POSTS_COUNT)
   end
 end

spec/components/timeline_lookup_spec.rb

@@ -6,23 +6,22 @@ describe TimelineLookup do
     expect(TimelineLookup.build([])).to eq([])
   end
 
+  it "returns an empty array for if input is an array if post ids" do
+    expect(TimelineLookup.build([1, 2, 3])).to eq([])
+  end
+
   it "returns the lookup for a series of posts" do
-    result = TimelineLookup.build([[111, 1, 10], [222, 2, 9], [333, 3, 8]])
+    result = TimelineLookup.build([[111, 10], [222, 9], [333, 8]])
     expect(result).to eq([[1, 10], [2, 9], [3, 8]])
   end
 
   it "omits duplicate dates" do
-    result = TimelineLookup.build([[111, 1, 10], [222, 2, 10], [333, 3, 8]])
-    expect(result).to eq([[1, 10], [3, 8]])
-  end
-
-  it "respects holes in the post numbers" do
-    result = TimelineLookup.build([[111, 1, 10], [222, 12, 10], [333, 30, 8]])
+    result = TimelineLookup.build([[111, 10], [222, 10], [333, 8]])
     expect(result).to eq([[1, 10], [3, 8]])
   end
 
   it "respects a `max_values` setting" do
-    input = (1..100).map { |i| [1000 + i, i, 100 - i] }
+    input = (1..100).map { |i| [1000 + i, 100 - i] }
 
     result = TimelineLookup.build(input, 5)
     expect(result.size).to eq(5)
@@ -30,7 +29,7 @@ describe TimelineLookup do
   end
 
   it "respects an uneven `max_values` setting" do
-    input = (1..100).map { |i| [1000 + i, i, 100 - i] }
+    input = (1..100).map { |i| [1000 + i, 100 - i] }
 
     result = TimelineLookup.build(input, 3)
     expect(result.size).to eq(3)

spec/components/topic_view_spec.rb

@@ -557,9 +557,9 @@ describe TopicView do
 
     it 'should return the right columns' do
       expect(topic_view.filtered_post_stream).to eq([
-        [post.id, post.post_number, 0],
-        [post2.id, post2.post_number, 0],
-        [post3.id, post3.post_number, 0]
+        [post.id, 0],
+        [post2.id, 0],
+        [post3.id, 0]
       ])
     end
 
@@ -571,9 +571,9 @@ describe TopicView do
           TopicView.const_set("MEGA_TOPIC_POSTS_COUNT", 2)
 
           expect(topic_view.filtered_post_stream).to eq([
-            [post.id, post.post_number],
-            [post2.id, post2.post_number],
-            [post3.id, post3.post_number]
+            post.id,
+            post2.id,
+            post3.id
           ])
         ensure
           TopicView.send(:remove_const, "MEGA_TOPIC_POSTS_COUNT")
@@ -582,4 +582,13 @@ describe TopicView do
       end
     end
   end
+
+  describe '#last_read_post_id' do
+    it 'should return the right id' do
+      post = Fabricate(:post, topic: topic)
+
+      expect(topic_view.last_read_post_id(nil)).to eq(nil)
+      expect(topic_view.last_read_post_id(post.post_number)).to eq(post.id)
+    end
+  end
 end