Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #2009 (closed defect: fixed)

Opened 3 years ago

Last modified 2 years ago

destroy and has_many/belongs_to not setting foriegn key value to NULL

Reported by: robbyrussell Assigned to: David
Priority: normal Milestone:
Component: ActiveRecord Version: 0.13.1
Severity: major Keywords: has_many, belongs_to, associations
Cc:

Description

class Order < ActiveRecord::Base
  belongs_to :customer
end
 
class Customer < ActiveRecord::Base
  has_many :orders
end

# some data

test_dev=# SELECT * FROM customers;  
 id |      name      
----+----------------
  1 | Robby
  2 | Nigel
  3 | robby on rails
(3 rows)
 
test_dev=# SELECT * FROM orders;
 id | customer_id | amount 
----+-------------+--------
  1 |           1 |  12.00
  2 |           3 |  12.00
(2 rows)

# CONSOLE

RobbyOnRails:~/Programming/footest robbyrussell$ ./script/console 
Loading development environment.
 
>> cust = Customer.find(3)
=> #<Customer:0x2751ce8 @attributes={"name"=>"robby on rails", "id"=>"3"}>
 
>> cust.orders
=> [#<Order:0x274e4d0 @attributes={"id"=>"2", "amount"=>"12.00", "customer_id"=>"3"}>]
 
>> Customer.destroy(3)
ActiveRecord::StatementInvalid: ERROR:  update or delete on "customers" violates foreign key constraint "orders_customer_id_fkey" on "orders"
DETAIL:  Key (id)=(3) is still referenced from table "orders".
:             DELETE FROM customers
            WHERE id = 3
 
        from /opt/local/lib/ruby/gems/1.8/gems/activerecord-1.11.1/lib/active_record/connection_adapters/abstract_adapter.rb:462:in `log'        from /opt/local/lib/ruby/gems/1.8/gems/activerecord-1.11.1/lib/active_record/connection_adapters/postgresql_adapter.rb:109:in `execute'
        from /opt/local/lib/ruby/gems/1.8/gems/activerecord-1.11.1/lib/active_record/connection_adapters/postgresql_adapter.rb:113:in `delete'
        from /opt/local/lib/ruby/gems/1.8/gems/activerecord-1.11.1/lib/active_record/base.rb:972:in `destroy_without_callbacks'
        from /opt/local/lib/ruby/gems/1.8/gems/activerecord-1.11.1/lib/active_record/callbacks.rb:321:in `destroy_without_transactions'
        from /opt/local/lib/ruby/gems/1.8/gems/activerecord-1.11.1/lib/active_record/transactions.rb:124:in `destroy'
        from /opt/local/lib/ruby/gems/1.8/gems/activerecord-1.11.1/lib/active_record/transactions.rb:124:in `transaction'
        from /opt/local/lib/ruby/gems/1.8/gems/activerecord-1.11.1/lib/active_record/transactions.rb:93:in `transaction'
        from /opt/local/lib/ruby/gems/1.8/gems/activerecord-1.11.1/lib/active_record/transactions.rb:120:in `transaction'
        from /opt/local/lib/ruby/gems/1.8/gems/activerecord-1.11.1/lib/active_record/transactions.rb:124:in `destroy'
        from /opt/local/lib/ruby/gems/1.8/gems/activerecord-1.11.1/lib/active_record/base.rb:420:in `destroy'
        from (irb):3
 
>> Customer.delete(3)
ActiveRecord::StatementInvalid: ERROR:  update or delete on "customers" violates foreign key constraint "orders_customer_id_fkey" on "orders"
DETAIL:  Key (id)=(3) is still referenced from table "orders".
: DELETE FROM customers WHERE id IN (3) 
        from /opt/local/lib/ruby/gems/1.8/gems/activerecord-1.11.1/lib/active_record/connection_adapters/abstract_adapter.rb:462:in `log'        from /opt/local/lib/ruby/gems/1.8/gems/activerecord-1.11.1/lib/active_record/connection_adapters/postgresql_adapter.rb:109:in `execute'
        from /opt/local/lib/ruby/gems/1.8/gems/activerecord-1.11.1/lib/active_record/connection_adapters/postgresql_adapter.rb:113:in `delete'
        from /opt/local/lib/ruby/gems/1.8/gems/activerecord-1.11.1/lib/active_record/base.rb:445:in `delete_all'
        from /opt/local/lib/ruby/gems/1.8/gems/activerecord-1.11.1/lib/active_record/base.rb:414:in `delete'
        from (irb):4
>> 

# with dependent

class Customer < ActiveRecord::Base
  has_many :orders, :dependent => true
end

This fixed that problem.


Okay, so now I will drop my table constraint and test this without the dependent. It should set the order.customer_id to null, right?

test_dev# \d orders
                                Table "public.orders"
   Column    |     Type      |                       Modifiers                        
-------------+---------------+--------------------------------------------------------
 id          | integer       | not null default nextval('public.orders_id_seq'::text)
 customer_id | integer       | 
 amount      | numeric(10,2) | 
Indexes:
    "orders_pkey" PRIMARY KEY, btree (id)
Foreign-key constraints:
    "orders_customer_id_fkey" FOREIGN KEY (customer_id) REFERENCES customers(id)
 
test_dev=# ALTER TABLE orders DROP CONSTRAINT orders_customer_id_fkey;
ALTER TABLE 
 
RobbyOnRails:~/Programming/footest robbyrussell$ ./script/console 
Loading development environment.
>> cust = Customer.create(:name => 'Jim')
=> #<Customer:0x275373c @new_record_before_save=true, @new_record=false, @attributes={"name"=>"Jim", "id"=>5}, @errors=#<ActiveRecord::Errors:0x274fa88 @base=#<Customer:0x275373c ...>, @errors={}>>
>> cust.orders.create(:amount => '25.00')
=> #<Order:0x274991c @new_record=false, @attributes={"id"=>4, "amount"=>"25.00", "customer_id"=>5}, @errors=#<ActiveRecord::Errors:0x2746dfc @base=#<Order:0x274991c ...>, @errors={}>>
>>                
 
 
test_dev=# SELECT * FROM orders;
 id | customer_id | amount 
----+-------------+--------
  1 |           1 |  12.00
  3 |           4 |  29.00
  4 |           5 |  25.00
(3 rows)

Okay, now if I destroy that last customer, it should set customer_id to null, right?

RobbyOnRails:~/Programming/footest robbyrussell$ ./script/console 
Loading development environment.
>> Customer.destroy(5)
=> {"name"=>"Jim", "id"=>"5"}
>>     
 
=# SELECT * FROM orders;
 id | customer_id | amount 
----+-------------+--------
  1 |           1 |  12.00
  3 |           4 |  29.00
  4 |           5 |  25.00
(3 rows)
 

Okay, so I got the :dependent part. However, in figuring this out, I found that Active Record is not setting the value of customer_id to NULL when I delete that specific customer from the customers table. This is in PostgreSQL... this leaves me with some semi-bad data when I have a table relationship that doesn't require the use of 'dependent'

Attachments

.2 (0 bytes) - added by robbyrussell on 08/18/05 20:44:19.
patch for associations.rb
has_many_belongs_to_fix.diff (0.7 kB) - added by robbyrussell on 08/18/05 20:44:36.
patch for associations.rb
has_many_belongs_to_fix_v2.diff (0.8 kB) - added by robbyrussell on 08/18/05 21:22:44.
oops, that last one had problem! ;-)
associations_dependent_1.diff (2.7 kB) - added by robbyrussell on 08/19/05 19:28:53.
association.rb diff

Change History

08/18/05 18:35:54 changed by Robby Russell

Okay, the problem in a nutshell. I pasted some other stuff... that shouldnt have been part of this. (the constraint portion)

basically, when you do a Model.destroy(id) on a model that has_many, it leaves the records in the other table with the now non-existent forieng key value.

RobbyOnRails:~/Programming/footest robbyrussell$ ./script/console 
Loading development environment.
>> Customer.destroy(5)
=> {"name"=>"Jim", "id"=>"5"}
>>     
 
=# SELECT * FROM orders;
 id | customer_id | amount 
----+-------------+--------
  1 |           1 |  12.00
  3 |           4 |  29.00
  4 |           5 |  25.00
(3 rows)

08/18/05 20:44:19 changed by robbyrussell

  • attachment .2 added.

patch for associations.rb

08/18/05 20:44:36 changed by robbyrussell

  • attachment has_many_belongs_to_fix.diff added.

patch for associations.rb

08/18/05 20:45:26 changed by Robby Russell

Okay, in associations.rb, it appears that it is not doing anything when dependent = false and :exclusively_dependent = false. looks like maybe someone didn't add an else to handle this task upon destory.

See attachment (has_many_belongs_to_fix.diff) for working fix.

08/18/05 21:22:44 changed by robbyrussell

  • attachment has_many_belongs_to_fix_v2.diff added.

oops, that last one had problem! ;-)

08/18/05 21:23:30 changed by robbyrussell

OOPS!

The last patch would update all records to NULL. No good! Can you delete a patch from the website?

Anyhow, try the _v2 version.

08/18/05 21:25:05 changed by robbyrussell

Some test info:

  Customer Load (0.007738)   SELECT * FROM customers WHERE customers.id = 17 LIMIT 1
  SQL (0.045302)   BEGIN
  SQL (0.009570)    SELECT a.attname, format_type(a.atttypid, a.atttypmod), d.adsrc
 FROM pg_attribute a LEFT JOIN pg_attrdef d
 ON a.attrelid = d.adrelid AND a.attnum = d.adnum
 WHERE a.attrelid = 'customers'::regclass
 AND a.attnum > 0 AND NOT a.attisdropped
 ORDER BY a.attnum

  Order Update (0.003257)   UPDATE orders SET customer_id = NULL WHERE customer_id = 17 
  Customer Destroy (0.001352)    DELETE FROM customers
 WHERE id = 17

  SQL (0.003138)   COMMIT                                          

08/19/05 12:17:17 changed by robbyrussell

Was talking with bitsweat about this. Should this be the default behavior of AR with has_many/belongs_to ?

I suggested that we have an option to pass has_many to handle the children.

He suggested,

has_many :dependent => :nullify

and

has_many :dependent => :destroy

... and of course, support '=> true' for backwards compatibility.

08/19/05 12:18:02 changed by robbyrussell

I'll see if I can whip up a patch later for this.

08/19/05 19:28:53 changed by robbyrussell

  • attachment associations_dependent_1.diff added.

association.rb diff

08/19/05 19:33:10 changed by robbyrussell

Okay,

I have uploaded: http://dev.rubyonrails.org/attachment/ticket/2009/associations_dependent_1.diff

This patch adds the ability to pass :dependent the following values

has_many :orders, :dependent => :nullify

and

has_many :orders, :dependent => :destroy

Passing true (old way) will still also work. Not passing a :dependent will just leave the values the way they were. (as it is now)

08/19/05 20:22:39 changed by robbyrussell

  • keywords changed from has_many, belongs_to to has_many, belongs_to, associations.

Okay, I submitted Ticket #2015 to resolve this.

09/08/05 10:38:47 changed by david

  • status changed from new to closed.
  • resolution set to fixed.

Super-seeded by #2015

08/06/06 01:04:28 changed by anonymous

home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home home