From 4945e3926b03c2727c46179cb12bc9c925757985 Mon Sep 17 00:00:00 2001 From: Aidan Date: Tue, 9 Jan 2024 09:34:41 -0500 Subject: [PATCH 1/2] functional changes --- lib/replica_pools/query_cache.rb | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/lib/replica_pools/query_cache.rb b/lib/replica_pools/query_cache.rb index 6a64059..a001bfb 100644 --- a/lib/replica_pools/query_cache.rb +++ b/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 From 9a9e3316e8829406832d8b864fdd730272862246 Mon Sep 17 00:00:00 2001 From: Aidan Date: Tue, 9 Jan 2024 13:01:52 -0500 Subject: [PATCH 2/2] add test --- spec/query_cache_spec.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/spec/query_cache_spec.rb b/spec/query_cache_spec.rb index 0c914c2..51135d8 100644 --- a/spec/query_cache_spec.rb +++ b/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