Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

ActiveRecord 4.0.1 regression: joining the same table multiple times #12770

Closed
johanneswuerbach opened this Issue · 13 comments

9 participants

@johanneswuerbach

When using a table multiple times in a query, AR 4.0.1 adds conditions specified as default scope to the INNER JOIN without aliasing the table name.

ActiveRecord 4.0.1 (INNER JOIN ... AND jobs.deleted_at):

SELECT COUNT(*) FROM "employees" INNER JOIN "jobs" "jobs_employees" ON "jobs_employees"."employee_id" = "employees"."id" AND "jobs"."deleted_at" IS NULL INNER JOIN "jobs" ON "employees"."id" = "jobs"."employee_id" WHERE "jobs"."deleted_at" IS NULL AND "jobs"."employer_id" = $1

ActiveRecord 4.0.0 (no condition):

SELECT COUNT(*) FROM "employees" INNER JOIN "jobs" "jobs_employees" ON "jobs_employees"."employee_id" = "employees"."id" INNER JOIN "jobs" ON "employees"."id" = "jobs"."employee_id" WHERE "jobs"."deleted_at" IS NULL AND "jobs"."employer_id" = $1

Sqlite3 ignores the invalid reference, but Postgres throws an error:

ActiveRecord::StatementInvalid: PG::UndefinedTable: ERROR:  invalid reference to FROM-clause entry for table "jobs"
LINE 1: ...s_employees"."employee_id" = "employees"."id" AND "jobs"."de...
                                                             ^
HINT:  Perhaps you meant to reference the table alias "jobs_employees".

Script to reproduce:

unless File.exist?('Gemfile')
  File.write('Gemfile', <<-GEMFILE)
    source 'https://rubygems.org'
    gem 'rails', github: 'rails/rails'
    gem 'pg'
  GEMFILE

  system 'bundle'
end

require 'bundler'
Bundler.setup(:default)

require 'active_record'
require 'minitest/autorun'
require 'logger'

ActiveRecord::Base.establish_connection adapter: 'postgresql', database: 'postgres', host: 'localhost'
ActiveRecord::Base.connection.drop_database 'multi-join' rescue nil
ActiveRecord::Base.connection.create_database 'multi-join'
ActiveRecord::Base.connection.close
ActiveRecord::Base.establish_connection adapter: 'postgresql', database: 'multi-join', host: 'localhost'

ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table "employers", force: true do |t|
  end
  create_table "employees", force: true do |t|
  end
  create_table "jobs", force: true do |t|
    t.integer "employer_id", null: false
    t.integer "employee_id", null: false
    t.datetime "deleted_at"
  end
end

class Employer < ActiveRecord::Base
  has_many :jobs
  has_many :employees, through: :jobs
end

class Employee < ActiveRecord::Base
  has_many :jobs
  has_many :employers, through: :jobs
end

class Job < ActiveRecord::Base
  belongs_to :employer
  belongs_to :employee
  default_scope { where(deleted_at: nil) } 
end

class BugTest < MiniTest::Unit::TestCase
  def test_multi_join
    employer = Employer.create
    assert_equal 0, employer.employees.joins(:jobs).count
  end
end

The above script is working fine using rails 4.0.0.

@johanneswuerbach

The issue was introduced by b407839 as the default scope was ignored before.
\cc @jonleighton

@al2o3cr

I've also run into this with something like Blog.includes(:posts, :comments).references(:posts, :comments) where Post has a default_scope. It's even more odd there, since there's no syntax error because the posts_blogs_join' condition referencespeople` instead BUT the resulting query does NOT return the same rows.

@al2o3cr

Definitely agree with @johanneswuerbach - this is related to that patch. In particular, it appears that build_default_scope isn't respecting the aliased table name - it always builds on relation instead of ActiveRecord::Relation.new(reflection.klass, table) as the other scope-invocations there do.

@nixme

fyi: This is still an issue in 4-1-STABLE

/cc @jonleighton @tenderlove

@tenderlove
Owner
@nixme

Just tested:
1. Generates correct SQL on 3-2-STABLE, same as 4.0.0.
2. Works fine with an in-memory sqlite DB. I believe it's as @johanneswuerbach mentioned, sqlite is okay with the early reference, but pg is not.

@al2o3cr

I've got a fix for this - will PR it today or tomorrow.

@tenderlove
Owner

@al2o3cr awesome, thanks. Please cc me on it!

@rafaelfranca rafaelfranca modified the milestone: 4.0.5, 4.0.4
@rafaelfranca rafaelfranca added regression and removed regression labels
@rafaelfranca rafaelfranca modified the milestone: 4.0.6, 4.0.5
@tenderlove tenderlove closed this issue from a commit
@al2o3cr al2o3cr Pass a base relation to build_default_scope when joining
This allows the default scope to be built using the current table alias.
Resolves #12770
70a5e56
@al2o3cr al2o3cr referenced this issue from a commit in al2o3cr/rails
@al2o3cr al2o3cr Pass a base relation to build_default_scope when joining
This allows the default scope to be built using the current table alias.
Resolves #12770
0f73bc9
@abstractcoder

any chance of this making it back into 4.0? I needed to regress production app to 4.0.0 to resolve this issue.

@rafaelfranca
Owner

Is not easy to backport #14154 to 4-0-stable since we had a lot of refactoring on master but if you want to give it a try please go ahead.

@al2o3cr

@abstractcoder - as @rafaelfranca mentioned, the backport is a bit tricky. Should have it working this weekend.

@rafaelfranca rafaelfranca modified the milestone: 4.0.6, 4.0.7
@ttosch ttosch referenced this issue from a commit
@al2o3cr al2o3cr Pass a base relation to build_default_scope when joining
This allows the default scope to be built using the current table alias.
Resolves #12770
74d94d3
@vlewin vlewin referenced this issue from a commit in vlewin/paranoia
@vlewin vlewin fix ActiveRecord::StatementInvalid: PG::UndefinedTable: ERROR: invali…
…d reference to FROM-clause ..., use table name alias (rails/rails#12770)
1421d0a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.