ByteMe
ByteMe

Reputation: 1392

How to avoid N+1 Queries for this code

I've been struggling to figure out how to avoid using so many SQL queries from this code. I'm taking in Collect objects via an API with Shopify, and then checking if there's a match already in my local database, and if not then creating a new local Collect object with parameters from my local matching Product and Collection. I wasn't sure how I would use includes or join because I'm doing a lookup and not just getting a value inside of the loop. Any pointers are welcomed.

  def seed_collects!
    shop = self.with_shopify!
    c = (1..(ShopifyAPI::Collect.count.to_f/250.0).ceil).flat_map do |page|
      ShopifyAPI::Collect.find(:all, :params => {:page => page.to_i, :limit => 150})
    end
    c.each do |collect|
      next if Collect.find_by(shopify_collect_id: collect.id)
      product = Product.find_by(shopify_product_id: collect.product_id.to_s)
      collection = Collection.find_by(shopify_collection_id: collect.collection_id.to_s)
      col = Collect.new(shopify_collect_id: collect.id,
                        position: collect.position,
                        product_id: product.id,
                        collection_id: collection.id)  
      col.save
      puts col.errors.inspect if col.errors.any?
    end
  end

Schema

  create_table "collections", force: :cascade do |t|
    t.string   "title"
    t.string   "shopify_collection_id"
    t.string   "collection_type"
    t.datetime "created_at",            null: false
    t.datetime "updated_at",            null: false
    t.integer  "shop_id"
  end

  create_table "collects", force: :cascade do |t|
    t.string   "position"
    t.string   "shopify_collect_id"
    t.datetime "created_at",         null: false
    t.datetime "updated_at",         null: false
    t.integer  "collection_id"
    t.integer  "product_id"
  end

  create_table "products", force: :cascade do |t|
    t.string   "title"
    t.string   "shopify_product_id"
    t.string   "product_type"
    t.string   "tags"
    t.datetime "created_at",         null: false
    t.datetime "updated_at",         null: false
    t.integer  "shop_id"
    t.string   "main_image_src"
  end

Models

class Collect < ActiveRecord::Base
  belongs_to :product
  belongs_to :collection

  delegate :shop, to: :product
  delegate :shopify_product_id, to: :product 
  delegate :shopify_collection_id, to: :collection 
end

class Collection < ActiveRecord::Base
  # verify all items belong to shop
  belongs_to :shop #verify
  has_many :collects, dependent: :destroy
  has_many :products, through: :collects

end

class Product < ActiveRecord::Base
  belongs_to :shop
  has_many :variants, ->{ order(:created_at) }, dependent: :destroy
  has_many :price_tests, dependent: :destroy
  has_many :metrics, ->{ order(:created_at) }, dependent: :destroy 
  has_many :collects, dependent: :destroy
  has_many :collections, through: :collects

Upvotes: 1

Views: 97

Answers (1)

Tai
Tai

Reputation: 1254

The better way to avoid N+1 in your case is to bulk-fetch collects, products, collections. Please try below code:

def seed_collects!
  shop = self.with_shopify!
  c = (1..(ShopifyAPI::Collect.count.to_f/250.0).ceil).flat_map do |page|
    ShopifyAPI::Collect.find(:all, :params => {:page => page.to_i, :limit => 150})
  end

  shopify_collect_ids    = c.map(&:id)
  shopify_product_ids    = c.map(&:product_id)
  shopify_collection_ids = c.map(&:collection_id)

  collects    = Collect.where(shopify_collect_id: shopify_collect_ids).pluck(:shopify_collect_id, :id)
  collect_ids = Hash[collects]

  products    = Product.where(shopify_product_id: shopify_product_ids).pluck(:shopify_product_id, :id)
  product_ids = Hash[products]

  collections    = Collection.where(shopify_collection_id: shopify_collection_ids).pluck(:shopify_collection_ids, :id)
  collection_ids = Hash[collections]

  c.each do |collect|
    next if collect_ids[collect.id]
    col = Collect.new(shopify_collect_id: collect.id,
                      position: collect.position,
                      product_id: product_ids[collect.product_id.to_s],
                      collection_id: collection_ids[collect.collection_id.to_s])  
    col.save
    puts col.errors.inspect if col.errors.any?
  end
end

Upvotes: 2

Related Questions