Shopify / Shopify/identity_cache
Cannot fetch_multi_by polymorphic field
Nobody has claimed this yet.
- Dominant language
- Ruby
- Stars
- 2k
- Forks
- 174
- Avg merge
- 13m
- Merged PRs (30d)
- 3
Description
I have made a new rails app with these models on the latest master of identity_cache:
models/item.rb
class Item < ApplicationRecord
include IdentityCache
belongs_to :widget, polymorphic: true
cache_belongs_to :widget
cache_index :widget
end
models/foo_widget.rb
class FooWidget < ApplicationRecord
include IdentityCache
has_many :items, as: :widget
cache_has_many :items, inverse: :widget
end
models/bar_widget.rb
class BarWidget < ApplicationRecord
include IdentityCache
has_many :items, as: :widget
cache_has_many :items, inverse: :widget
end
schema:
class AddTables < ActiveRecord::Migration[5.2]
def change
create_table :items do |t|
t.bigint :widget_id, null: false
t.string :widget_type, null: false
end
create_table :foo_widgets
create_table :bar_widgets
add_index :items, %i[widget_type widget_id]
end
end
I would like to be able to run this:
widgets = FooWidget.fetch_multi([1, 2, 3])
items = Item.fetch_multi_by_widget(widgets)
But, that generates an invalid SQL query:
ActiveRecord::StatementInvalid (SQLite3::SQLException: no such column: widget: SELECT widget, "items"."id" FROM "items" WHERE "items"."widget_type" = ? AND "items"."widget_id" IN (?, ?))
I can't change my cache_index to cache_index :widget_id, :widget_type because fetch_multi_by methods are only generated for cache_indexes with only one field.
I used to be able to pass in arrays into the fetch_by_widget_id_and_subject_type methods in 0.5.1, but now that doesn't work on master (e.g. an array of widget_ids generates a SQL query with NULL as the widget_id value). I guess was never the way those methods were intended to be used, but now I don't have a way to bulk fetch Items in a way that won't make N+1 queries when the cache is missed.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the polymorphic association and cache_index declarations in models/item.rb, models/foo_widget.rb, and models/bar_widget.rb, then trace the fetch_multi_by_widget entry point shown in the report. Compare this with the older fetch_by_widget_id_and_subject_type behavior described. Done means bulk-fetching items for FooWidget records produces valid SQL without N+1 cache-miss queries.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rails, ruby
- Domain
- backend, database
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100