Question

Just briefly, I have run into a dreaded 2(n) queries problem. If n = the number of skills in the database, then my characters#edit form will take 2(n) queries to load the page. It will SELECT a PlayerSkill (the join table) once every skill, and it will look up the Skill once per skill.

Here is some code which I believe is pertinent to the situation. In essence, the models, views, and controllers involved in this process, less the model validations and less the actions I'm not concerned about.

The controller:

  # GET /characters/1/edit
  def edit
    @character = Character.find(params[:id], :include => {:player_skills => :skill})
    stub_player_skills
  end

  private
    def stub_player_skills
      @skills = Skill.find(:all)
      @skills.each do |skill|
        if (skill.player_skills.empty?)
          ps = @character.player_skills.build(:skill_id => skill.id, :name => skill.name)
        end
      end
    end

The model(s):

class Character < ActiveRecord::Base
  belongs_to :user
  belongs_to :campaign
  has_many :sheets, :dependent => :destroy
  has_many :tokens, :dependent => :destroy

  has_many :player_skills, :dependent => :destroy
  has_many :skills, :through => :player_skills
  accepts_nested_attributes_for :player_skills, :allow_destroy => true
end

The offending view (HAML):

%h1
  Editing Character

- form_for @character do |f|
  = f.error_messages
  %p
    = f.label :name
    %br
    = f.text_field :name
  %p
    = f.label :race
    %br
    = f.text_field :race
  %p
    = f.label :char_class
    %br
    = f.text_field :char_class
  %p
    -f.fields_for :player_skills do |ps|
      =ps.object.skill.name
      =ps.text_field :level
      =ps.hidden_field :skill_id
      -unless ps.object.new_record?
        =ps.check_box '_destroy'
        =ps.label '_destroy', 'Remove'
      %br
  %p
    = f.submit

My understanding of the situation is that eager loading exists to grab the association in (roughly) a single extra query.

I need to properly apply eager loading in two areas, and I am just at a loss regarding how to do it.

In the stub_player_skills method, it needs to create a PlayerSkill object assuming the character does not already have one. It could benefit from eager loading here because it loops through each skill in the database. This is where the first "n-queries" are coming from.

Then on the view, fields_for loops through all the PlayerSkills we've racked up, because there is no way to eager load here, when I call =ps.object.skill.name to print out the skill's name, it does a Skill lookup, which brings in the second set of "n-queries."

My primary concern lies in the view layer, I cannot find any documentation (Rails API or otherwise) that states how you could eager load the associations if you're using fields_for to generate a nested form.

Thanks for any and all responses :) ~Robbie

Was it helpful?

Solution

Can you try this and see if it will work?

You can keep your model the way it is.

Your controller can then look like this

def edit
  # Get all the skill objects once only
  skills = Skill.find(:all)

  # Used to extract Skill#name
  skills_hash = {}
  skills.map { |s| skills_hash[s.id] = s.name }

  # Create an array of the skill-ids
  skill_ids = skills.map { |s| s.id }

  @character = Character.find(params[:id])

  # Determine the character's missing skills
  skill_ids -= @character.player_skill_ids

  # Build all of the missing skills
  skill_ids.each do |id|
    @character.player_skills.build(:skill_id => id, :name => skills_hash[id])
  end
end

OTHER TIPS

In case anyones interested in my "final" solution to this problem:

I've resorted to storing an array of the skill names, and referencing it in the view via a counter, as seen here:

  %p
    - index = 0
    -f.fields_for :player_skills do |ps|
      =@skill_arr[index]
      =ps.text_field :level
      =ps.hidden_field :skill_id
      -unless ps.object.new_record?
        =ps.check_box '_destroy'
        =ps.label '_destroy', 'Remove'
      - index += 1
      %br

In the controller, I've moved almost all the logic to the stub_player_skills method where it belongs, and taking a page out of Coderama's book, I've come up with this:

  private
    def stub_player_skills
      @skills = Skill.find(:all)
      @skills.each do |skill|
        skill_exists = @character.player_skills.select do |i|
          i.skill_id == skill.id
        end
        if skill_exists.empty?
          ps = @character.player_skills.build(:skill_id => skill.id, :name => skill.name)
        end
      end

      @skill_arr = @character.player_skills.map do |el|
        el.name.nil? ? el.skill.name : el.name
      end
    end

In the model layer, I just had to :include => :skill on the has_many :through relationship to get rid of a few more queries.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top