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

Wrong SQL for handle :dependent => :nullify objects that have default_scope with ordering on many tables #8217

Closed
Envek opened this issue Nov 14, 2012 · 13 comments

Comments

@Envek
Copy link
Contributor

Envek commented Nov 14, 2012

Description

If dependent model have default_scope with both includes and order methods (with a string SQL ORDER BY clause, that references to included model table), when deleting parent model (with has_many :children, :dependent => :nullify), Rails tries to nullify dependent model foreign keys with SQL query, that respects order method and doesn't respects includes (and/or joins).

Example (gives missing FROM-clause entry for table "buildings" in PostgreSQL):

UPDATE "rooms" SET "department_id" = NULL WHERE "rooms"."id" IN (SELECT "rooms"."id" FROM "rooms"  WHERE "rooms"."department_id" = 1 AND "rooms"."id" IN (1) ORDER BY rooms.name ASC, buildings.name ASC)

Expected behaviour

  1. Use unscoped subquery (at least don't use ordering, IMO it's absolutely meaningless there) or
  2. Try to use all methods from default_scope

Environment

Have tested on:

  1. Rails 3.2.9 on Ruby 1.9.3 (test app)
  2. Rails 3.2.2 on Ruby EE (real app)

How to reproduce

Create rails app:

rails new test_app -d postgresql
cd test_app/
echo "gem 'therubyracer', :platforms => :ruby" >> Gemfile
echo "gem 'debugger'" >> Gemfile
bundle update
rails g model Building name:string
rails g model Department name:string
rails g model Room name:string building:references department:references
rake db:create
rake db:setup

Assume that models are looks like this (note the default_scope):

class Building < ActiveRecord::Base
  has_many :rooms, :dependent => :destroy
  attr_accessible :name
  validates :name, :presence => true
end

class Department < ActiveRecord::Base
  has_many :rooms, :dependent => :nullify
  attr_accessible :name
  validates :name, :presence => true
end

class Room < ActiveRecord::Base
  belongs_to :building
  belongs_to :department
  attr_accessible :name, :department, :building
  validates :name, :presence => true
  validates :building_id, :presence => true
  default_scope includes(:building).order("rooms.name ASC, buildings.name ASC")
end

In rails console create some buildings, rooms and departments:

Building.create(name: "Main")
Department.create(name: "IT")
Room.create(name: "1", building: Building.first, department: Department.first)

And try to delete one department:

1.9.3p194 :014 > Department.first.destroy
  Department Load (1.6ms)  SELECT "departments".* FROM "departments" LIMIT 1
   (0.9ms)  BEGIN
  SQL (2.9ms)  SELECT "rooms"."id" AS t0_r0, "rooms"."name" AS t0_r1, "rooms"."building_id" AS t0_r2, "rooms"."department_id" AS t0_r3, "rooms"."created_at" AS t0_r4, "rooms"."updated_at" AS t0_r5, "buildings"."id" AS t1_r0, "buildings"."name" AS t1_r1, "buildings"."created_at" AS t1_r2, "buildings"."updated_at" AS t1_r3 FROM "rooms" LEFT OUTER JOIN "buildings" ON "buildings"."id" = "rooms"."building_id" WHERE "rooms"."department_id" = 1 ORDER BY rooms.name ASC, buildings.name ASC
  SQL (2.8ms)  UPDATE "rooms" SET "department_id" = NULL WHERE "rooms"."id" IN (SELECT "rooms"."id" FROM "rooms" WHERE "rooms"."department_id" = 1 AND "rooms"."id" IN (1) ORDER BY rooms.name ASC, buildings.name ASC)
PG::Error: ERROR:  missing FROM-clause entry for table "buildings"
LINE 1: ... AND "rooms"."id" IN (1) ORDER BY rooms.name ASC, buildings....
                                                             ^
: UPDATE "rooms" SET "department_id" = NULL WHERE "rooms"."id" IN (SELECT "rooms"."id" FROM "rooms"  WHERE "rooms"."department_id" = 1 AND "rooms"."id" IN (1) ORDER BY rooms.name ASC, buildings.name ASC)
   (1.4ms)  ROLLBACK
ActiveRecord::StatementInvalid: PG::Error: ERROR:  missing FROM-clause entry for table "buildings"
LINE 1: ... AND "rooms"."id" IN (1) ORDER BY rooms.name ASC, buildings....
                                                             ^
: UPDATE "rooms" SET "department_id" = NULL WHERE "rooms"."id" IN (SELECT "rooms"."id" FROM "rooms"  WHERE "rooms"."department_id" = 1 AND "rooms"."id" IN (1) ORDER BY rooms.name ASC, buildings.name ASC)
@steveklabnik
Copy link
Member

Hey, thanks for the great reproduction steps. These kinds of bugs make my head hurt, so I'm not sure what's right here, just wanted to let you know I've read it.

@Envek
Copy link
Contributor Author

Envek commented Jan 24, 2013

Reading Active Record changes in Rails 4, I was wondering that references query method could solve this issue like this:

default_scope -> { includes(:building).order("rooms.name ASC, buildings.name ASC").references(:building) }

But no, it doesn't. This issue is present in rails 4 too (today's master).

@stouset
Copy link
Contributor

stouset commented Apr 9, 2013

@Envek Can you confirm whether or not this problem persists in Rails 4?

@Envek
Copy link
Contributor Author

Envek commented Apr 10, 2013

Yes, it persists. I've checked it on both 4.0.0.beta1 gem and rails/rails@24b1d4fc63dc commit checkout from master with and without call to references.

@neerajdotname
Copy link

@Envek

Can you post a gist as per the section 1.2 of Rails guide http://edgeguides.rubyonrails.org/contributing_to_ruby_on_rails.html#create-a-self-contained-gist-for-active-record-issues?

I'll take a look. Thanks.

@neerajdotname
Copy link

@Envek if you change default_scope from

default_scope includes(:building).order("rooms.name ASC, buildings.name ASC")

to

default_scope joins(:building).order("rooms.name ASC, buildings.name ASC")

then the problem goes away.

Here is the gist I created to test this issue. https://gist.github.com/neerajdotname/5356529

The episode of railscasts has detailed info about joins vs includes http://railscasts.com/episodes/22-eager-loading-revised?autoplay=true.

@Envek
Copy link
Contributor Author

Envek commented Apr 11, 2013

@neerajdotname, I know, it fixes. But sadly joins generate an INNER JOIN, but I need a LEFT OUTER JOIN, that generated by includes call. (It's weird but Room can belong to no Building).

My test case gist: https://gist.github.com/Envek/5360783

@neerajdotname
Copy link

@Envek thanks for the gist. I'll take a look.

@JonRowe
Copy link
Contributor

JonRowe commented May 5, 2013

Hey did you ever get a chance to take a look at this @neerajdotname? (Just triaging ;)

@neerajdotname
Copy link

@JonRowe I'll look into it this week.

@JonRowe
Copy link
Contributor

JonRowe commented May 6, 2013

❤️

@rafaelfranca
Copy link
Member

This issue has been automatically marked as stale because it has not been commented on for at least
three months.

The resources of the Rails team are limited, and so we are asking for your help.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

@rails-bot
Copy link

This issue has been automatically closed because of inactivity.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

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

7 participants