rails / solid_cache

A database-backed ActiveSupport::Cache::Store
MIT License
890 stars 63 forks source link

Rails Edge breaking change with insert_all #164

Closed ron-shinall closed 8 months ago

ron-shinall commented 8 months ago

Since https://github.com/rails/rails/commit/1c0982d88a83bc733d7334accdd075b97b5db98c has been merged, activerecord/lib/active_record/insert_all.rb now expects connection to be passed in as an argument.

Current code:

def upsert_all_no_query_cache(payloads)
  insert_all = ActiveRecord::InsertAll.new(
    self,
    add_key_hash_and_byte_size(payloads),
    unique_by: upsert_unique_by,
    on_duplicate: :update,
    update_only: upsert_update_only
  )
  sql = connection.build_insert_sql(ActiveRecord::InsertAll::Builder.new(insert_all))

  message = +"#{self} "
  message << "Bulk " if payloads.many?
  message << "Upsert"
  # exec_query_method does not clear the query cache, exec_insert_all does
  connection.send exec_query_method, sql, message
end

For Rails Edge, it would need to be changed to:

def upsert_all_no_query_cache(payloads)
  insert_all = ActiveRecord::InsertAll.new(
    self,
    connection,
    add_key_hash_and_byte_size(payloads),
    unique_by: upsert_unique_by,
    on_duplicate: :update,
    update_only: upsert_update_only
  )
  sql = connection.build_insert_sql(ActiveRecord::InsertAll::Builder.new(insert_all))

  message = +"#{self} "
  message << "Bulk " if payloads.many?
  message << "Upsert"
  # exec_query_method does not clear the query cache, exec_insert_all does
  connection.send exec_query_method, sql, message
end

I am happy to work on a PR for this, but I would need some guidance on the desired approach in dealing with the different Rails versions within this gem. I don't suspect we'd want an if condition inside the upsert_all_no_query_cache method.

ron-shinall commented 8 months ago

One idea:

def upsert_all_no_query_cache(payloads)
  insert_all_args = [ self,
                      Rails.version >= "7.2" ? connection : nil,
                      add_key_hash_and_byte_size(payloads) ].compact
  options = { unique_by: upsert_unique_by,
              on_duplicate: :update,
              update_only: upsert_update_only }
  insert_all = ActiveRecord::InsertAll.new(*insert_all_args, **options)
  sql = connection.build_insert_sql(ActiveRecord::InsertAll::Builder.new(insert_all))

  message = +"#{self} "
  message << "Bulk " if payloads.many?
  message << "Upsert"
  # exec_query_method does not clear the query cache, exec_insert_all does
  connection.send exec_query_method, sql, message
end