Merge pull request #50 from kickstarter/update_select_all

Update `select_all` for Rails 7.1

lib/replica_pools/query_cache.rb

@@ -18,29 +18,20 @@ module QueryCache
     # select_all is trickier. it needs to use the leader
     # connection for cache logic, but ultimately pass its query
     # through to whatever connection is current.
-    def select_all(*args, **kwargs)
-      relation, name, raw_binds = args
-
-      if !query_cache_enabled || locked?(relation)
-        return route_to(current, :select_all, *args, **kwargs)
+    def select_all(arel, name = nil, binds = [], *args, preparable: nil, **kwargs)
+      if !query_cache_enabled || locked?(arel)
+        return route_to(current, :select_all, arel, name, binds, *args, preparable: preparable, **kwargs)
       end
 
-      # duplicate binds_from_relation behavior introduced in 4.2.
-      if raw_binds.blank? && relation.is_a?(ActiveRecord::Relation)
-        arel, binds = relation.arel, relation.bind_values
-      else
-        arel, binds = relation, Array(raw_binds)
+      # duplicate arel_from_relation behavior introduced in 5.2.
+      if arel.is_a?(ActiveRecord::Relation)
+        arel = arel.arel
       end
 
-      sql = to_sql(arel, binds)
-
-      args[0] = sql
-      args[2] = binds
+      sql, binds, preparable = to_sql_and_binds(arel, binds, preparable)
 
-      if Gem::Version.new(ActiveRecord.version) < Gem::Version.new('5.1')
-        cache_sql(sql, binds) { route_to(current, :select_all, *args, **kwargs) }
-      else
-        cache_sql(sql, name, binds) { route_to(current, :select_all, *args, **kwargs) }
+      cache_sql(sql, name, binds) do
+        route_to(current, :select_all, arel, name, binds, *args, preparable: preparable, **kwargs)
       end
     end
 

spec/query_cache_spec.rb

@@ -116,4 +116,16 @@ def executor
       expect(TestModel.pluck(:id).count).to eql TestModel.all.count
     end
   end
+
+  describe '.ids regression test' do
+    it 'should work with query caching' do
+      TestModel.connection.enable_query_cache!
+      expect(TestModel.ids.count).to eql TestModel.all.count
+    end
+
+    it 'should work if query cache is not enabled' do
+      TestModel.connection.disable_query_cache!
+      expect(TestModel.ids.count).to eql TestModel.all.count
+    end
+  end
 end