From f06bb0bd6cd03429ec3056ab0411c1135a7d6da9 Mon Sep 17 00:00:00 2001 From: Oliver Morgan Date: Wed, 3 Jul 2024 12:14:46 +0100 Subject: [PATCH 01/14] fix unprepared statements --- app/models/solid_cache/entry.rb | 6 +++++- test/unit/behaviors/cache_store_behavior.rb | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/models/solid_cache/entry.rb b/app/models/solid_cache/entry.rb index a3d3752..20ebcd9 100644 --- a/app/models/solid_cache/entry.rb +++ b/app/models/solid_cache/entry.rb @@ -116,7 +116,11 @@ def get_all_sql(key_hashes) @get_all_sql_binds ||= {} @get_all_sql_binds[key_hashes.count] ||= build_sql(where(key_hash: key_hashes).select(:key, :value)) else - @get_all_sql_no_binds ||= build_sql(where(key_hash: [ 1, 2 ]).select(:key, :value)).gsub("?, ?", "?") + if keys.size == 1 + @get_all_sql_no_binds_one_key ||= build_sql(where(key_hash: 1).select(:key, :value)) + else + @get_all_sql_no_binds_many_keys ||= build_sql(where(key_hash: [ 1, 2 ]).select(:key, :value)).gsub("?, ?", "?") + end end end diff --git a/test/unit/behaviors/cache_store_behavior.rb b/test/unit/behaviors/cache_store_behavior.rb index fdf50b4..8666c92 100644 --- a/test/unit/behaviors/cache_store_behavior.rb +++ b/test/unit/behaviors/cache_store_behavior.rb @@ -145,6 +145,7 @@ def test_read_multi @cache.write(other_key, "baz") @cache.write(SecureRandom.alphanumeric, "biz") assert_equal({ key => "bar", other_key => "baz" }, @cache.read_multi(key, other_key)) + assert_equal({ key => "bar" }, @cache.read_multi(key)) end def test_read_multi_empty_list From ba32f8311e506d93511429341387c737fce37437 Mon Sep 17 00:00:00 2001 From: Oliver Morgan Date: Wed, 3 Jul 2024 12:17:59 +0100 Subject: [PATCH 02/14] keys => key_hashes --- app/models/solid_cache/entry.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/solid_cache/entry.rb b/app/models/solid_cache/entry.rb index 20ebcd9..b89570a 100644 --- a/app/models/solid_cache/entry.rb +++ b/app/models/solid_cache/entry.rb @@ -116,7 +116,7 @@ def get_all_sql(key_hashes) @get_all_sql_binds ||= {} @get_all_sql_binds[key_hashes.count] ||= build_sql(where(key_hash: key_hashes).select(:key, :value)) else - if keys.size == 1 + if key_hashes.size == 1 @get_all_sql_no_binds_one_key ||= build_sql(where(key_hash: 1).select(:key, :value)) else @get_all_sql_no_binds_many_keys ||= build_sql(where(key_hash: [ 1, 2 ]).select(:key, :value)).gsub("?, ?", "?") From 13d69c5d996b3dcaa563197380f5ede428c32843 Mon Sep 17 00:00:00 2001 From: Oliver Morgan Date: Wed, 3 Jul 2024 13:00:41 +0100 Subject: [PATCH 03/14] fix tests --- Rakefile | 2 +- test/dummy/config/database.yml | 8 ++++++++ .../config/solid_cache_unprepared_statements.yml | 12 ++++++++++++ test/unit/behaviors/cache_store_behavior.rb | 1 - 4 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 test/dummy/config/solid_cache_unprepared_statements.yml diff --git a/Rakefile b/Rakefile index 167e23c..403d497 100644 --- a/Rakefile +++ b/Rakefile @@ -23,7 +23,7 @@ def run_without_aborting(*tasks) end def configs - [ :default, :connects_to, :database, :no_database, :shards ] + [ :default, :connects_to, :database, :no_database, :shards, :unprepared_statements ] end task :test do diff --git a/test/dummy/config/database.yml b/test/dummy/config/database.yml index 3f404ce..087a56d 100644 --- a/test/dummy/config/database.yml +++ b/test/dummy/config/database.yml @@ -42,6 +42,10 @@ development: primary: <<: *default database: <%= database("database_cache_development") %> + primary_unprepared_statements: + <<: *default + database: <%= database("database_cache_development") %> + prepared_statements: false primary_replica: <<: *default database: <%= database("database_cache_development") %> @@ -67,6 +71,10 @@ test: primary: <<: *default database: <%= database("database_cache_test") %> + primary_unprepared_statements: + <<: *default + database: <%= database("database_cache_development") %> + prepared_statements: false primary_replica: <<: *default database: <%= database("database_cache_test") %> diff --git a/test/dummy/config/solid_cache_unprepared_statements.yml b/test/dummy/config/solid_cache_unprepared_statements.yml new file mode 100644 index 0000000..87b471f --- /dev/null +++ b/test/dummy/config/solid_cache_unprepared_statements.yml @@ -0,0 +1,12 @@ +default: &default + database: primary_unprepared_statements + + store_options: + max_age: 3600 + max_size: + +development: + <<: *default + +test: + <<: *default diff --git a/test/unit/behaviors/cache_store_behavior.rb b/test/unit/behaviors/cache_store_behavior.rb index 8666c92..fdf50b4 100644 --- a/test/unit/behaviors/cache_store_behavior.rb +++ b/test/unit/behaviors/cache_store_behavior.rb @@ -145,7 +145,6 @@ def test_read_multi @cache.write(other_key, "baz") @cache.write(SecureRandom.alphanumeric, "biz") assert_equal({ key => "bar", other_key => "baz" }, @cache.read_multi(key, other_key)) - assert_equal({ key => "bar" }, @cache.read_multi(key)) end def test_read_multi_empty_list From 2b7cebd6f6530a7187abfcb949eb55350b892f51 Mon Sep 17 00:00:00 2001 From: Oliver Morgan Date: Wed, 3 Jul 2024 13:04:45 +0100 Subject: [PATCH 04/14] Create primary_unprepared_statements_schema.rb --- .../primary_unprepared_statements_schema.rb | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 test/dummy/db/primary_unprepared_statements_schema.rb diff --git a/test/dummy/db/primary_unprepared_statements_schema.rb b/test/dummy/db/primary_unprepared_statements_schema.rb new file mode 100644 index 0000000..1adf031 --- /dev/null +++ b/test/dummy/db/primary_unprepared_statements_schema.rb @@ -0,0 +1,25 @@ +# This file is auto-generated from the current state of the database. Instead +# of editing this file, please use the migrations feature of Active Record to +# incrementally modify your database, and then regenerate this schema definition. +# +# This file is the source Rails uses to define your schema when running `bin/rails +# db:schema:load`. When creating a new database, `bin/rails db:schema:load` tends to +# be faster and is potentially less error prone than running all of your +# migrations from scratch. Old migrations may fail to apply correctly if those +# migrations use external dependencies or application code. +# +# It's strongly recommended that you check this file into your version control system. + +ActiveRecord::Schema[7.0].define(version: 2024_01_10_111702) do + create_table "solid_cache_entries", force: :cascade do |t| + t.binary "key", limit: 1024, null: false + t.binary "value", limit: 536870912, null: false + t.datetime "created_at", null: false + t.integer "key_hash", limit: 8, null: false + t.integer "byte_size", limit: 4, null: false + t.index ["byte_size"], name: "index_solid_cache_entries_on_byte_size" + t.index ["key_hash", "byte_size"], name: "index_solid_cache_entries_on_key_hash_and_byte_size" + t.index ["key_hash"], name: "index_solid_cache_entries_on_key_hash", unique: true + end + +end From 3bd7510614205d9456b5bb8802fb1a4b103b98a7 Mon Sep 17 00:00:00 2001 From: Oliver Morgan Date: Wed, 3 Jul 2024 13:29:26 +0100 Subject: [PATCH 05/14] exclude some tests --- test/models/solid_cache/record_test.rb | 4 ++-- test/test_helper.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/models/solid_cache/record_test.rb b/test/models/solid_cache/record_test.rb index d426374..e7d569b 100644 --- a/test/models/solid_cache/record_test.rb +++ b/test/models/solid_cache/record_test.rb @@ -7,7 +7,7 @@ class RecordTest < ActiveSupport::TestCase test "each_shard" do shards = SolidCache::Record.each_shard.map { SolidCache::Record.current_shard } case ENV["SOLID_CACHE_CONFIG"] - when "config/solid_cache_no_database.yml", "config/solid_cache_database.yml" + when "config/solid_cache_no_database.yml", "config/solid_cache_database.yml", "config/solid_cache_unprepared_statements.yml" assert_equal [ :default ], shards when "config/solid_cache_connects_to.yml", "config/solid_cache_shards.yml", nil assert_equal [ :primary_shard_one, :primary_shard_two, :secondary_shard_one, :secondary_shard_two ], shards @@ -19,7 +19,7 @@ class RecordTest < ActiveSupport::TestCase test "each_shard uses the default role" do role = ActiveRecord::Base.connected_to(role: :reading) { SolidCache::Record.each_shard.map { SolidCache::Record.current_role } } case ENV["SOLID_CACHE_CONFIG"] - when "config/solid_cache_no_database.yml", "config/solid_cache_database.yml" + when "config/solid_cache_no_database.yml", "config/solid_cache_database.yml", "config/solid_cache_unprepared_statements.yml" assert_equal [ :reading ], role when "config/solid_cache_connects_to.yml", "config/solid_cache_shards.yml", nil assert_equal [ :writing, :writing, :writing, :writing ], role diff --git a/test/test_helper.rb b/test/test_helper.rb index faa71be..862f0f4 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -82,6 +82,6 @@ def second_shard_key end def single_database? - [ "config/solid_cache_database.yml", "config/solid_cache_no_database.yml" ].include?(ENV["SOLID_CACHE_CONFIG"]) + [ "config/solid_cache_database.yml", "config/solid_cache_no_database.yml", "config/solid_cache_unprepared_statements.yml" ].include?(ENV["SOLID_CACHE_CONFIG"]) end end From 3e083de0e7a151f204813c2b8041aaa06cd5e1c1 Mon Sep 17 00:00:00 2001 From: Oliver Morgan Date: Wed, 3 Jul 2024 13:32:04 +0100 Subject: [PATCH 06/14] remove unnecessary schema --- .../primary_unprepared_statements_schema.rb | 25 ------------------- 1 file changed, 25 deletions(-) delete mode 100644 test/dummy/db/primary_unprepared_statements_schema.rb diff --git a/test/dummy/db/primary_unprepared_statements_schema.rb b/test/dummy/db/primary_unprepared_statements_schema.rb deleted file mode 100644 index 1adf031..0000000 --- a/test/dummy/db/primary_unprepared_statements_schema.rb +++ /dev/null @@ -1,25 +0,0 @@ -# This file is auto-generated from the current state of the database. Instead -# of editing this file, please use the migrations feature of Active Record to -# incrementally modify your database, and then regenerate this schema definition. -# -# This file is the source Rails uses to define your schema when running `bin/rails -# db:schema:load`. When creating a new database, `bin/rails db:schema:load` tends to -# be faster and is potentially less error prone than running all of your -# migrations from scratch. Old migrations may fail to apply correctly if those -# migrations use external dependencies or application code. -# -# It's strongly recommended that you check this file into your version control system. - -ActiveRecord::Schema[7.0].define(version: 2024_01_10_111702) do - create_table "solid_cache_entries", force: :cascade do |t| - t.binary "key", limit: 1024, null: false - t.binary "value", limit: 536870912, null: false - t.datetime "created_at", null: false - t.integer "key_hash", limit: 8, null: false - t.integer "byte_size", limit: 4, null: false - t.index ["byte_size"], name: "index_solid_cache_entries_on_byte_size" - t.index ["key_hash", "byte_size"], name: "index_solid_cache_entries_on_key_hash_and_byte_size" - t.index ["key_hash"], name: "index_solid_cache_entries_on_key_hash", unique: true - end - -end From 2a529cbee83ec178e1a5f21a9ba098b4bc7030fe Mon Sep 17 00:00:00 2001 From: Oliver Morgan Date: Wed, 3 Jul 2024 15:44:51 +0100 Subject: [PATCH 07/14] typo --- test/dummy/config/database.yml | 2 +- .../primary_unprepared_statements_schema.rb | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 test/dummy/db/primary_unprepared_statements_schema.rb diff --git a/test/dummy/config/database.yml b/test/dummy/config/database.yml index 087a56d..793bf42 100644 --- a/test/dummy/config/database.yml +++ b/test/dummy/config/database.yml @@ -73,7 +73,7 @@ test: database: <%= database("database_cache_test") %> primary_unprepared_statements: <<: *default - database: <%= database("database_cache_development") %> + database: <%= database("database_cache_test") %> prepared_statements: false primary_replica: <<: *default diff --git a/test/dummy/db/primary_unprepared_statements_schema.rb b/test/dummy/db/primary_unprepared_statements_schema.rb new file mode 100644 index 0000000..1adf031 --- /dev/null +++ b/test/dummy/db/primary_unprepared_statements_schema.rb @@ -0,0 +1,25 @@ +# This file is auto-generated from the current state of the database. Instead +# of editing this file, please use the migrations feature of Active Record to +# incrementally modify your database, and then regenerate this schema definition. +# +# This file is the source Rails uses to define your schema when running `bin/rails +# db:schema:load`. When creating a new database, `bin/rails db:schema:load` tends to +# be faster and is potentially less error prone than running all of your +# migrations from scratch. Old migrations may fail to apply correctly if those +# migrations use external dependencies or application code. +# +# It's strongly recommended that you check this file into your version control system. + +ActiveRecord::Schema[7.0].define(version: 2024_01_10_111702) do + create_table "solid_cache_entries", force: :cascade do |t| + t.binary "key", limit: 1024, null: false + t.binary "value", limit: 536870912, null: false + t.datetime "created_at", null: false + t.integer "key_hash", limit: 8, null: false + t.integer "byte_size", limit: 4, null: false + t.index ["byte_size"], name: "index_solid_cache_entries_on_byte_size" + t.index ["key_hash", "byte_size"], name: "index_solid_cache_entries_on_key_hash_and_byte_size" + t.index ["key_hash"], name: "index_solid_cache_entries_on_key_hash", unique: true + end + +end From c8221c979a7f178dab5c5027d817159656314ca4 Mon Sep 17 00:00:00 2001 From: Oliver Morgan Date: Wed, 3 Jul 2024 16:48:23 +0100 Subject: [PATCH 08/14] remove prepared_statements switch --- app/models/solid_cache/entry.rb | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/app/models/solid_cache/entry.rb b/app/models/solid_cache/entry.rb index b89570a..4f4b31f 100644 --- a/app/models/solid_cache/entry.rb +++ b/app/models/solid_cache/entry.rb @@ -112,16 +112,8 @@ def get_sql end def get_all_sql(key_hashes) - if connection.prepared_statements? - @get_all_sql_binds ||= {} - @get_all_sql_binds[key_hashes.count] ||= build_sql(where(key_hash: key_hashes).select(:key, :value)) - else - if key_hashes.size == 1 - @get_all_sql_no_binds_one_key ||= build_sql(where(key_hash: 1).select(:key, :value)) - else - @get_all_sql_no_binds_many_keys ||= build_sql(where(key_hash: [ 1, 2 ]).select(:key, :value)).gsub("?, ?", "?") - end - end + @get_all_sql_binds ||= {} + @get_all_sql_binds[key_hashes.count] ||= build_sql(where(key_hash: key_hashes).select(:key, :value)) end def build_sql(relation) @@ -135,12 +127,12 @@ def build_sql(relation) def select_all_no_query_cache(query, values) uncached do - if connection.prepared_statements? - result = connection.select_all(sanitize_sql(query), "#{name} Load", Array(values), preparable: true) - else - result = connection.select_all(sanitize_sql([ query, values ]), "#{name} Load", Array(values), preparable: false) + binds = Array(values).map do |value| + ActiveRecord::Relation::QueryAttribute.new("key_hash", value, SolidCache::Entry.type_for_attribute("key_hash")) end + result = connection.select_all(sanitize_sql(query), "#{name} Load", binds, preparable: true) + result.cast_values(SolidCache::Entry.attribute_types) end end From e957b5b1426fcd932708a387a690a4396af77f90 Mon Sep 17 00:00:00 2001 From: Oliver Morgan Date: Wed, 3 Jul 2024 17:02:36 +0100 Subject: [PATCH 09/14] go back to what what it was before --- app/models/solid_cache/entry.rb | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/app/models/solid_cache/entry.rb b/app/models/solid_cache/entry.rb index 4f4b31f..d25e1cd 100644 --- a/app/models/solid_cache/entry.rb +++ b/app/models/solid_cache/entry.rb @@ -112,8 +112,12 @@ def get_sql end def get_all_sql(key_hashes) - @get_all_sql_binds ||= {} - @get_all_sql_binds[key_hashes.count] ||= build_sql(where(key_hash: key_hashes).select(:key, :value)) + if connection.prepared_statements? + @get_all_sql_binds ||= {} + @get_all_sql_binds[key_hashes.count] ||= build_sql(where(key_hash: key_hashes).select(:key, :value)) + else + @get_all_sql_no_binds ||= build_sql(where(key_hash: [ 1, 2 ]).select(:key, :value)).gsub("?, ?", "?") + end end def build_sql(relation) @@ -127,12 +131,12 @@ def build_sql(relation) def select_all_no_query_cache(query, values) uncached do - binds = Array(values).map do |value| - ActiveRecord::Relation::QueryAttribute.new("key_hash", value, SolidCache::Entry.type_for_attribute("key_hash")) + if connection.prepared_statements? + result = connection.select_all(sanitize_sql(query), "#{name} Load", Array(values), preparable: true) + else + result = connection.select_all(sanitize_sql([ query, Array(values) ]), "#{name} Load", Array(values), preparable: false) end - result = connection.select_all(sanitize_sql(query), "#{name} Load", binds, preparable: true) - result.cast_values(SolidCache::Entry.attribute_types) end end From 820f8ddbf6d9183fc25ffab40c8c08776aaaa642 Mon Sep 17 00:00:00 2001 From: Oliver Morgan Date: Wed, 3 Jul 2024 17:15:39 +0100 Subject: [PATCH 10/14] all the way back --- app/models/solid_cache/entry.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/solid_cache/entry.rb b/app/models/solid_cache/entry.rb index d25e1cd..3b76dc3 100644 --- a/app/models/solid_cache/entry.rb +++ b/app/models/solid_cache/entry.rb @@ -134,7 +134,7 @@ def select_all_no_query_cache(query, values) if connection.prepared_statements? result = connection.select_all(sanitize_sql(query), "#{name} Load", Array(values), preparable: true) else - result = connection.select_all(sanitize_sql([ query, Array(values) ]), "#{name} Load", Array(values), preparable: false) + result = connection.select_all(sanitize_sql([ query, values ]), "#{name} Load", binds, preparable: false) end result.cast_values(SolidCache::Entry.attribute_types) From 4f07a826942771d8a7ad5a2ef446924692130049 Mon Sep 17 00:00:00 2001 From: Oliver Morgan Date: Wed, 3 Jul 2024 17:23:33 +0100 Subject: [PATCH 11/14] Update entry.rb --- app/models/solid_cache/entry.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/solid_cache/entry.rb b/app/models/solid_cache/entry.rb index 3b76dc3..9949eb3 100644 --- a/app/models/solid_cache/entry.rb +++ b/app/models/solid_cache/entry.rb @@ -134,7 +134,7 @@ def select_all_no_query_cache(query, values) if connection.prepared_statements? result = connection.select_all(sanitize_sql(query), "#{name} Load", Array(values), preparable: true) else - result = connection.select_all(sanitize_sql([ query, values ]), "#{name} Load", binds, preparable: false) + result = connection.select_all(sanitize_sql([ query, values ]), "#{name} Load", values, preparable: false) end result.cast_values(SolidCache::Entry.attribute_types) From 6c4c234e16af0b48f83196513dc54d4e2ace01ca Mon Sep 17 00:00:00 2001 From: Oliver Morgan Date: Wed, 3 Jul 2024 17:25:24 +0100 Subject: [PATCH 12/14] Update entry.rb --- app/models/solid_cache/entry.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/solid_cache/entry.rb b/app/models/solid_cache/entry.rb index 9949eb3..a3d3752 100644 --- a/app/models/solid_cache/entry.rb +++ b/app/models/solid_cache/entry.rb @@ -134,7 +134,7 @@ def select_all_no_query_cache(query, values) if connection.prepared_statements? result = connection.select_all(sanitize_sql(query), "#{name} Load", Array(values), preparable: true) else - result = connection.select_all(sanitize_sql([ query, values ]), "#{name} Load", values, preparable: false) + result = connection.select_all(sanitize_sql([ query, values ]), "#{name} Load", Array(values), preparable: false) end result.cast_values(SolidCache::Entry.attribute_types) From b8c5c1745291e0e0961ce2b9ed99efc80d1b499b Mon Sep 17 00:00:00 2001 From: Oliver Morgan Date: Wed, 3 Jul 2024 17:49:44 +0100 Subject: [PATCH 13/14] Update entry.rb --- app/models/solid_cache/entry.rb | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/app/models/solid_cache/entry.rb b/app/models/solid_cache/entry.rb index a3d3752..a37fc0b 100644 --- a/app/models/solid_cache/entry.rb +++ b/app/models/solid_cache/entry.rb @@ -112,12 +112,8 @@ def get_sql end def get_all_sql(key_hashes) - if connection.prepared_statements? - @get_all_sql_binds ||= {} - @get_all_sql_binds[key_hashes.count] ||= build_sql(where(key_hash: key_hashes).select(:key, :value)) - else - @get_all_sql_no_binds ||= build_sql(where(key_hash: [ 1, 2 ]).select(:key, :value)).gsub("?, ?", "?") - end + @get_all_sql_binds ||= {} + @get_all_sql_binds[key_hashes.count] ||= build_sql(where(key_hash: key_hashes).select(:key, :value)) end def build_sql(relation) @@ -134,7 +130,7 @@ def select_all_no_query_cache(query, values) if connection.prepared_statements? result = connection.select_all(sanitize_sql(query), "#{name} Load", Array(values), preparable: true) else - result = connection.select_all(sanitize_sql([ query, values ]), "#{name} Load", Array(values), preparable: false) + result = connection.select_all(sanitize_sql([ query, *values ]), "#{name} Load", Array(values), preparable: false) end result.cast_values(SolidCache::Entry.attribute_types) From b5570f4e5fed4e3af6a3c9470ae9f35fd0a865a9 Mon Sep 17 00:00:00 2001 From: Oliver Morgan Date: Mon, 22 Jul 2024 20:03:38 +0100 Subject: [PATCH 14/14] rename get_all_sql_binds --- app/models/solid_cache/entry.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/solid_cache/entry.rb b/app/models/solid_cache/entry.rb index a37fc0b..a8aef18 100644 --- a/app/models/solid_cache/entry.rb +++ b/app/models/solid_cache/entry.rb @@ -112,8 +112,8 @@ def get_sql end def get_all_sql(key_hashes) - @get_all_sql_binds ||= {} - @get_all_sql_binds[key_hashes.count] ||= build_sql(where(key_hash: key_hashes).select(:key, :value)) + @get_all_sql ||= {} + @get_all_sql[key_hashes.count] ||= build_sql(where(key_hash: key_hashes).select(:key, :value)) end def build_sql(relation)