Cannot fetch_multi_by polymorphic field

Open
#392 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
42/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Stale
Tech stack
rails, ruby
Domain
backend, database

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.

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.

Dominant language
Ruby
Stars
2k
Forks
174
Avg merge
13m
Merged PRs (30d)
3

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from Shopify/identity_cache

All issues in Shopify/identity_cache

Similar issues

More Ruby issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.