Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preloading has_many through associations loads duplicated records #39073

Closed
inkstak opened this issue Apr 28, 2020 · 3 comments · Fixed by #39156
Closed

Preloading has_many through associations loads duplicated records #39073

inkstak opened this issue Apr 28, 2020 · 3 comments · Fixed by #39156

Comments

@inkstak
Copy link

inkstak commented Apr 28, 2020

Steps to reproduce

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "rails", "6.0.2.2"
  gem "sqlite3"
end

require "active_record"
require "minitest/autorun"
require "logger"

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :entities, force: true do |t|
  end

  create_table :memberships, force: true do |t|
    t.integer :parent_id
    t.integer :child_id
  end

  create_table :documents, force: true do |t|
    t.integer :entity_id
  end
end

class Entity < ActiveRecord::Base
  has_many :memberships, foreign_key: :parent_id
  has_many :children,    through: :memberships
  has_many :documents

  has_one  :parent_membership, class_name: 'Membership', foreign_key: :child_id
  has_one  :parent,            through: :parent_membership
  has_many :shared_documents,  through: :parent, source: :documents
end

class Membership < ActiveRecord::Base
  belongs_to :parent, class_name: 'Entity'
  belongs_to :child,  class_name: 'Entity'
end

class Document < ActiveRecord::Base
  belongs_to :customer
end

class BugTest < Minitest::Test
  def test_association_stuff
    parent = Entity.create!
    parent.documents.create!

    3.times do
      parent.children << Entity.create!
    end

    # Each parent and child entities have only one document
    assert_equal 1, Entity.preload(:shared_documents)[1].shared_documents.size

    # Loading the parent alongside the documents will return 3 documents
    assert_equal 1, Entity.preload(:parent, :shared_documents)[1].shared_documents.size
  end
end

I found two similar issues : #37891 and #39035.
Both use different types of associations and the internal ActiveRecord::Associations::Preloader
The example above uses only public API.

Expected behavior

In the example above, each parent and children have only one document or shared document
Preloading the parent should not change this behavior.

Actual behavior

Preloading the parent alongside shared documents duplicates the returned documents

System configuration

Rails version: 6.0.2.2

Ruby version: 2.6.3

@alipman88
Copy link
Contributor

alipman88 commented Apr 28, 2020

I'm able to reproduce this, and it seems like unexpected behavior at first glance.

Here's a slightly more minimal test case (using less relations) demonstrating the same phenomena:

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "rails", github: "rails/rails"
  gem "sqlite3"
end

require "active_record"
require "minitest/autorun"
require "logger"

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :meetings, force: true do |t|
  end

  create_table :participants, force: true do |t|
    t.integer :meeting_id
  end

  create_table :agenda_items, force: true do |t|
    t.integer :meeting_id
  end
end

class Meeting < ActiveRecord::Base
  has_many :participants
  has_many :agenda_items
end

class Participant < ActiveRecord::Base
  belongs_to :meeting
  has_many :agenda_items, through: :meeting
end

class AgendaItem < ActiveRecord::Base
  belongs_to :meeting
end

class BugTest < Minitest::Test
  def test_association_stuff
    meeting = Meeting.create
    meeting.agenda_items << AgendaItem.create

    3.times do
      meeting.participants << Participant.create
    end

    association_without_agenda_items_preloaded = Participant.preload(:meeting)
    association_with_agenda_items_preloaded = Participant.preload(:meeting, :agenda_items)

    assert_equal association_without_agenda_items_preloaded[0].agenda_items.size,
      association_with_agenda_items_preloaded[0].agenda_items.size
  end
end

@alipman88
Copy link
Contributor

I'm noticing that my test passes in Rails 5.2.4.2, but fails when ran against the current master branch.

So, I'd categorize this as a bug/regression.

@alipman88
Copy link
Contributor

It looks like Rails' behavior has changed back and forth several times. Using git-bisect, I believe 2847653 was the most recent commit to modify behavior to the current state (duplicates), and prior to that 5f9e050 (no duplicates).

@bogdan, any interest in investigating? You've tackled a few preloading-related improvements. (If nobody else takes this on, I'll plan on picking this up in a few weeks, once I settle some other PRs I'm working on.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants