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

Adding an includes to a query in Rails 5.2 adds a LEFT OUTER JOIN that breaks with pluck #32701

Closed
troym9731 opened this issue Apr 23, 2018 · 8 comments

Comments

@troym9731
Copy link

troym9731 commented Apr 23, 2018

The below test case passes on Rails 5.1.4. Let me know if I'm missing something obvious in the release notes that would explain why this would fail in 5.2 😄

Steps to reproduce

# frozen_string_literal: true

begin
  require "bundler/inline"
rescue LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
  raise e
end

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

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

  gem "rails", '5.2.0'
  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 :roles, force: true do |t|
    t.string :name
  end

  create_table :users, force: true do |t|
    t.integer :role_id
  end

  create_table :payments, force: true do |t|
    t.integer :user_id
  end
end

class Role < ActiveRecord::Base
  has_many :users
end

class User < ActiveRecord::Base
  has_many :payments
  belongs_to :role
end

class Payment < ActiveRecord::Base
  belongs_to :user
end

class BugTest < Minitest::Test
  def test_association_stuff
    role = Role.create!(name: 'Admin')
    user = User.create!(role: role)
    payment = Payment.create!(user: user)

    payment_ids_with_admin_user =
      Payment.joins(:user)
        .merge(User.joins(:role).where(roles: {name: 'Admin'}))
        .pluck('payments.id')

    assert_equal [payment.id], payment_ids_with_admin_user

    payment_ids_with_admin_user_and_includes =
      Payment.joins(:user)
        .merge(User.joins(:role).where(roles: {name: 'Admin'}))
        .includes(user: [:role])
        .pluck('payments.id')

    assert_equal [payment.id], payment_ids_with_admin_user_and_includes
  end
end

Expected behavior

Adding the includes shouldn't affect the resulting query.

Note: The pluck is needed to get this query to fail. Otherwise, it runs fine.

Actual behavior

Adding the includes adds an extra LEFT OUTER JOIN to the resulting query. With the pluck, it ends up throwing a "Not unique table/alias" error.

System configuration

Rails version: 5.2.0

Ruby version: 2.3.3

@rafaelfranca
Copy link
Member

Now that you have the script and the version where it passes it should not be hard to use git bisect to find which commit introduced this change. Can you try that? It would help us a lot.

@troym9731
Copy link
Author

Yup! Doing that right now.

@rafaelfranca
Copy link
Member

Thank you!

@troym9731
Copy link
Author

troym9731 commented Apr 23, 2018

This is the commit that broke the above test case: 249ddd0

@troym9731 troym9731 changed the title Adding an includes to a query in Rails 5.2 adds a LEFT OUTER JOIN unexpectedly Adding an includes to a query in Rails 5.2 adds a LEFT OUTER JOIN that breaks with pluck Apr 24, 2018
@GeminPatel
Copy link

I think this => #32856 is a duplicate

@manu-d
Copy link

manu-d commented May 14, 2018

The test case I presented in #32856 fails as well with v 5.1.4 unlike this one

@kamipo kamipo added the pinned label May 17, 2018
@eostrom
Copy link
Contributor

eostrom commented May 18, 2018

I have a simpler test case for a related error that I strongly suspect is caused by the same commit, although I haven't bisected. Gist: https://gist.github.com/eostrom/46ab1773b84e6c440f0ece028aa1e063

In my case, no includes or pluck is needed, and the relations are not nested but there's a reciprocal join. The heart of it is:

# Post has_many :comments
# Comment belongs_to :post
Post.joins(:comments).merge(Comment.joins(:post))

In Rails 5.1.6 (Ruby 2.5.1p57), this generates SQL that gives "Comment's posts" its own alias:

SELECT "posts".* FROM "posts"
  INNER JOIN "comments"
    ON "comments"."post_id" = "posts"."id"
  LEFT OUTER JOIN "posts" "posts_comments"
    ON "posts_comments"."id" = "comments"."post_id"

In Rails 5.2.0, it was changed to generate an INNER JOIN, but the alias was lost, so it raises a "not unique table/alias" error (or "ambiguous column name" in SQLite):

SELECT "posts".* FROM "posts"
  INNER JOIN "comments"
    ON "comments"."post_id" = "posts"."id"
  INNER JOIN "posts"
    ON "posts"."id" = "comments"."post_id"

(This is a contrived example, of course. In my real application, I'm building a complex scope on Comment that adds certain conditions, including "belonging to posts that have some characteristic." Elsewhere, I want to retrieve a list of posts that have those comments. Also, they're not posts and comments.)

Let me know if this should be filed as a separate issue.

@kamipo
Copy link
Member

kamipo commented Jun 11, 2018

Fixed by #33066.

@kamipo kamipo closed this as completed Jun 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants