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

[ci skip] correct for ActiveRecord::Associations::Preloader #20673

Merged
merged 1 commit into from Jun 23, 2015

Conversation

aditya-kapoor
Copy link
Contributor

I saw this example and something doesn't seemed right.

@senny
Copy link
Member

senny commented Jun 23, 2015

@aditya-kapoor please use the PR body to explain the change. I'm not sure I understand why i'ts necessary. As far as I can tell the example only use the has_many :books association. The inverse is not mentioned.

@aditya-kapoor
Copy link
Contributor Author

@senny I think there the belongs_to is missing in the example code.

If there was no belongs_to then query specified in the next lines Author.includes(:books).where(:name => ['bell hooks', 'Homer').to_a would not work.

Please correct if I am missing something.

@senny
Copy link
Member

senny commented Jun 23, 2015

@aditya-kapoor why should it not work?

# Activate the gem you are reporting the issue against.
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 :authors do |t|
    t.string :name
    t.integer :age
  end

  create_table :books do |t|
    t.string :title
    t.integer :sales
    t.belongs_to :author
  end
end

class Author < ActiveRecord::Base
  # columns: name, age
  has_many :books
end

class Book < ActiveRecord::Base
  # columns: title, sales
end

class ExampleTest < Minitest::Test
  def test_example
    Author.includes(:books).where(name: ['bell hooks', 'Homer']).to_a

    Author.includes(:books).where(books: {title: 'Illiad'}).to_a
  end
end

# == OUTPUT
# -- create_table(:authors)
# D, [2015-06-23T15:50:44.702986 #23785] DEBUG -- :    (7.6ms)  CREATE TABLE "authors" ("id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, "name" varchar, "age" integer)
#    -> 0.0184s
# -- create_table(:books)
# D, [2015-06-23T15:50:44.703782 #23785] DEBUG -- :    (0.2ms)  CREATE TABLE "books" ("id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, "title" varchar, "sales" integer, "author_id" integer)
#    -> 0.0006s
# Run options: --seed 34140

# # Running:

# D, [2015-06-23T15:50:44.813255 #23785] DEBUG -- :   Author Load (0.1ms)  SELECT "authors".* FROM "authors" WHERE "authors"."name" IN ('bell hooks', 'Homer')
# D, [2015-06-23T15:50:44.837158 #23785] DEBUG -- :   SQL (0.2ms)  SELECT "authors"."id" AS t0_r0, "authors"."name" AS t0_r1, "authors"."age" AS t0_r2, "books"."id" AS t1_r0, "books"."title" AS t1_r1, "books"."sales" AS t1_r2, "books"."author_id" AS t1_r3 FROM "authors" LEFT OUTER JOIN "books" ON "books"."author_id" = "authors"."id" WHERE "books"."title" = ?  [["title", "Illiad"]]
# .

# Finished in 0.044347s, 22.5494 runs/s, 0.0000 assertions/s.

# 1 runs, 0 assertions, 0 failures, 0 errors, 0 skips

@senny senny closed this Jun 23, 2015
@aditya-kapoor
Copy link
Contributor Author

@senny Usually it is the has_many and belongs_to arrangement which we have.
We don't have any knowledge of author_id inside the Book model. See the columns comment inside Book model.

PS you are explicitly specifying this in your migration, so why not in your model?

@senny senny reopened this Jun 23, 2015
@senny
Copy link
Member

senny commented Jun 23, 2015

@aditya-kapoor granted, the columns: comment could be extend with author_id. There's still no need for the belongs to though. I feel like this makes it more explicit what we are talking about. It's only related to the has_many and of course the right table layout.

@senny senny merged commit c5c0ace into rails:master Jun 23, 2015
senny added a commit that referenced this pull request Jun 23, 2015
[ci skip] correct for ActiveRecord::Associations::Preloader
@senny
Copy link
Member

senny commented Jun 23, 2015

Merged a modified version only expanding the columns: comment: (de0a2a4)

@aditya-kapoor
Copy link
Contributor Author

@senny Thank you for your time and explanation.

@senny
Copy link
Member

senny commented Jun 23, 2015

@aditya-kapoor thanks for being persistent and help us improve the docs ✨

senny added a commit that referenced this pull request Jun 23, 2015
[ci skip] correct for ActiveRecord::Associations::Preloader
@aditya-kapoor aditya-kapoor deleted the correct-preload-doc branch June 23, 2015 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants