Rails core - Jul 2006 - Why are has_one objects resaved when the parent is saved?

If the associated object of a has_one association has been loaded, it
is resaved when the parent is saved. Eg:

class Person < ActiveRecord::Base
  has_one :user
end

class User < ActiveRecord::Base
  belongs_to :person
end

p = Person.find(:first)
p.save # As expected, nothing happens with the user association
p.user # Loads the associated object
p.save # As well as saving the person, the user is saved as well.
Seems unnecessary.

Trac is down (or at least doesn''t work for me), so I''ve
attached a
patch that fixes the behaviour so that the child object is not saved
unnecessarily. Test included.

-Jonathan.

Forgot to attach the patch! Here it is ...

-Jonathan.

On 7/28/06, Jonathan Viney <jonathan.viney@gmail.com>
wrote:
> If the associated object of a has_one association has been loaded, it
> is resaved when the parent is saved. Eg:
>
> class Person < ActiveRecord::Base
>   has_one :user
> end
>
> class User < ActiveRecord::Base
>   belongs_to :person
> end
>
> p = Person.find(:first)
> p.save # As expected, nothing happens with the user association
> p.user # Loads the associated object
> p.save # As well as saving the person, the user is saved as well.
> Seems unnecessary.
>
> Trac is down (or at least doesn''t work for me), so I''ve
attached a
> patch that fixes the behaviour so that the child object is not saved
> unnecessarily. Test included.
>
> -Jonathan.
>

_______________________________________________
Rails-core mailing list
Rails-core@lists.rubyonrails.org
http://lists.rubyonrails.org/mailman/listinfo/rails-core

+1

On 7/27/06, Jonathan Viney <jonathan.viney@gmail.com>
wrote:
> Forgot to attach the patch! Here it is ...
>
> -Jonathan.

-- 
Rick Olson
http://techno-weenie.net

On 27-jul-2006, at 16:20, Jonathan Viney wrote:
> Forgot to attach the patch! Here it is ...
There''s a good reason for this:

p = Person.find :first
p.user.name = ''New Name''
p.save

Does the patch check to see if the ''p.user'' is modified or
only if it''s
new?  I only see checking the ''new_record'' attribute.

Manfred Stienstra wrote:
>
> On 27-jul-2006, at 16:20, Jonathan Viney wrote:
>
>> Forgot to attach the patch! Here it is ...
>
> There''s a good reason for this:
>
> p = Person.find :first
> p.user.name = ''New Name''
> p.save
>
> _______________________________________________
> Rails-core mailing list
> Rails-core@lists.rubyonrails.org
> http://lists.rubyonrails.org/mailman/listinfo/rails-core
>
-- 
Michael Genereux
SimianCodex - Web Application Development
949-874-6268

On 7/27/06, Michael Genereux <mgenereu@simiancodex.com>
wrote:
> Does the patch check to see if the ''p.user'' is modified
or only if it''s
> new?  I only see checking the ''new_record'' attribute.
ActiveRecord has no dirty field checking by default...

-- 
Rick Olson
http://techno-weenie.net

On Jul 27, 2006, at 7:47 AM, Manfred Stienstra wrote:
> On 27-jul-2006, at 16:20, Jonathan Viney wrote:
>
>> Forgot to attach the patch! Here it is ...
>
> There''s a good reason for this:
>
> p = Person.find :first
> p.user.name = ''New Name''
> p.save
That''s a valid point.  But did you read Jonathan''s patch?  It
covers
that case where you''re dealing with new records just fine.  I think  
the issue he was correcting was that has_one *always* saves the child  
object, not just for new records.

--
Josh Susser
http://blog.hasmanythrough.com

On 2006-07-27, at 11:52 , Michael Genereux wrote:
> Does the patch check to see if the ''p.user'' is modified
or only if
> it''s new?  I only see checking the ''new_record''
attribute.
Its does, here''s the full conditional:

if !association.nil? && (new_record? || association.new_record? ||  
association["#{reflection.primary_key_name}"] != id)

On 2006-07-27, at 11:56 , Caio Chassot wrote:
>
> On 2006-07-27, at 11:52 , Michael Genereux wrote:
>
>> Does the patch check to see if the ''p.user'' is
modified or only if
>> it''s new?  I only see checking the
''new_record'' attribute.
>
> Its does, here''s the full conditional:
>
> if !association.nil? && (new_record? || association.new_record? ||
> association["#{reflection.primary_key_name}"] != id)
Oops. Read: It doesn''t.

In otherwords, the patch is valid for cascade saving a new association 
but invalid for cascade updates to said association.

Rick Olson wrote:
> On 7/27/06, Michael Genereux <mgenereu@simiancodex.com> wrote:
>> Does the patch check to see if the ''p.user'' is
modified or only if it''s
>> new?  I only see checking the ''new_record'' attribute.
>
> ActiveRecord has no dirty field checking by default...
>
-- 
Michael Genereux
SimianCodex - Web Application Development
949-874-6268

Perhaps, but the same does not happen for any other association types.
has_one appears to be the only association that behaves as you
described. Also, that code would not work the other way around:

u = User.find(:first)
u.person.name = "New name"
u.save # person is not saved

-Jonathan.

On 7/28/06, Manfred Stienstra <manfred@gmail.com>
wrote:
>
> On 27-jul-2006, at 16:20, Jonathan Viney wrote:
>
> > Forgot to attach the patch! Here it is ...
>
> There''s a good reason for this:
>
> p = Person.find :first
> p.user.name = ''New Name''
> p.save
>
> _______________________________________________
> Rails-core mailing list
> Rails-core@lists.rubyonrails.org
> http://lists.rubyonrails.org/mailman/listinfo/rails-core
>

It doesn''t have dirty object checking either, right?

Rick Olson wrote:
> On 7/27/06, Michael Genereux <mgenereu@simiancodex.com> wrote:
>> Does the patch check to see if the ''p.user'' is
modified or only if it''s
>> new?  I only see checking the ''new_record'' attribute.
>
> ActiveRecord has no dirty field checking by default...
>
-- 
Michael Genereux
SimianCodex - Web Application Development
949-874-6268

On 2006-07-27, at 11:56 , Josh Susser wrote:
> That''s a valid point.  But did you read Jonathan''s patch?
It
> covers that case where you''re dealing with new records just fine.
> I think the issue he was correcting was that has_one *always* saves  
> the child object, not just for new records.
Strictly, it does not *always* save the child, only if it''s been  
*loaded*.

It checks the association ivar directly, meaning it does not go  
through the accessor, and therefore does not try to load it.

Saving loaded associations is a cheap way to make sure you save  
everytime a change *could have* happened. Without a dirtyness check,  
I''d say that''s good enough.

The alternative would be a handcoded dirtyness check, something like:

   assoc_record.class.find(record.id).attributes.all? { |(k, v)|
     assoc_record.send(k) == v }

Which quickly becomes expansive.

I''m curious to know if "dirty" checking was considered during
the design
process of AR. It would add performance benefits (configurable?) for larger
tables and associations in a write heavy environment especially if you have
text or blob fields in your tables. Anyone care to PDI this as a plugin?

Bob Silva
http://i.nfectio.us/


-----Original Message-----
From: rails-core-bounces@lists.rubyonrails.org
[mailto:rails-core-bounces@lists.rubyonrails.org] On Behalf Of Caio Chassot
Sent: Thursday, July 27, 2006 8:03 AM
To: rails-core@lists.rubyonrails.org
Subject: Re: [Rails-core] Re: Why are has_one objects resaved when the
parentis saved?


On 2006-07-27, at 11:56 , Josh Susser wrote:
> That''s a valid point.  But did you read Jonathan''s patch?
It
> covers that case where you''re dealing with new records just fine.
> I think the issue he was correcting was that has_one *always* saves  
> the child object, not just for new records.
Strictly, it does not *always* save the child, only if it''s been  
*loaded*.

It checks the association ivar directly, meaning it does not go  
through the accessor, and therefore does not try to load it.

Saving loaded associations is a cheap way to make sure you save  
everytime a change *could have* happened. Without a dirtyness check,  
I''d say that''s good enough.

The alternative would be a handcoded dirtyness check, something like:

   assoc_record.class.find(record.id).attributes.all? { |(k, v)|
     assoc_record.send(k) == v }

Which quickly becomes expansive.

_______________________________________________
Rails-core mailing list
Rails-core@lists.rubyonrails.org
http://lists.rubyonrails.org/mailman/listinfo/rails-core

On Fri, Jul 28, 2006 at 01:07:59PM -0700, Bob Silva
wrote:
> I''m curious to know if "dirty" checking was considered
during the design
> process of AR. It would add performance benefits (configurable?) for larger
> tables and associations in a write heavy environment especially if you have
> text or blob fields in your tables. Anyone care to PDI this as a plugin?
As I remember it, about a year ago Jeremy Kemper implemented dirty column
tracking  and decided against it for reasons I forget. Something wasn''t
right
in the cosmos though.

If I am remembering correctly, maybe he could share what the issues were
before someone dives into implementing a plugin for this.

marcel
-- 
Marcel Molina Jr. <marcel@vernix.org>

Given that even the most basic, multiuser apps hit concurrency issues 
with editing different fields in the same record, I was surprised that 
version locking was the only level of support Rails offered.

I would *love* to PDI field level change checking (I''ve done it before 
on another framework) and also updating Base to only UPDATE fields that 
have changed.  While it may sound funny, I''m a gung-ho person and would
just need to be pointed in the direction of how to make a plugin.  I 
wander through the ActiveRecord code all the time but not all of Rails.  
I was pondering adding support with a patch and have some really great 
ideas but if it didn''t get accepted, I didn''t want to waste
the time.
Also, how do I check to see if someone else hasn''t already started a 
plugin/gem/patch for the same issue?

Bob Silva wrote:
> I''m curious to know if "dirty" checking was considered
during the design
> process of AR. It would add performance benefits (configurable?) for larger
> tables and associations in a write heavy environment especially if you have
> text or blob fields in your tables. Anyone care to PDI this as a plugin?
>
> Bob Silva
> http://i.nfectio.us/
>
>
> -----Original Message-----
> From: rails-core-bounces@lists.rubyonrails.org
> [mailto:rails-core-bounces@lists.rubyonrails.org] On Behalf Of Caio Chassot
> Sent: Thursday, July 27, 2006 8:03 AM
> To: rails-core@lists.rubyonrails.org
> Subject: Re: [Rails-core] Re: Why are has_one objects resaved when the
> parentis saved?
>
>
> On 2006-07-27, at 11:56 , Josh Susser wrote:
>   
>> That''s a valid point.  But did you read Jonathan''s
patch?  It
>> covers that case where you''re dealing with new records just
fine.
>> I think the issue he was correcting was that has_one *always* saves  
>> the child object, not just for new records.
>>     
>
> Strictly, it does not *always* save the child, only if it''s been  
> *loaded*.
>
> It checks the association ivar directly, meaning it does not go  
> through the accessor, and therefore does not try to load it.
>
> Saving loaded associations is a cheap way to make sure you save  
> everytime a change *could have* happened. Without a dirtyness check,  
> I''d say that''s good enough.
>
> The alternative would be a handcoded dirtyness check, something like:
>
>    assoc_record.class.find(record.id).attributes.all? { |(k, v)|
>      assoc_record.send(k) == v }
>
> Which quickly becomes expansive.
>
> _______________________________________________
> Rails-core mailing list
> Rails-core@lists.rubyonrails.org
> http://lists.rubyonrails.org/mailman/listinfo/rails-core
>
> _______________________________________________
> Rails-core mailing list
> Rails-core@lists.rubyonrails.org
> http://lists.rubyonrails.org/mailman/listinfo/rails-core
>
>   
-- 
Michael Genereux
SimianCodex - Web Application Development
949-874-6268

> I would *love* to PDI field level change checking (I''ve done it
before
> on another framework) and also updating Base to only UPDATE fields that
> have changed.  While it may sound funny, I''m a gung-ho person and
would
> just need to be pointed in the direction of how to make a plugin.  I
> wander through the ActiveRecord code all the time but not all of Rails.
> I was pondering adding support with a patch and have some really great
> ideas but if it didn''t get accepted, I didn''t want to
waste the time.
> Also, how do I check to see if someone else hasn''t already started
a
> plugin/gem/patch for the same issue?
It''s more or less impossible to use old value /new value optimistic
locking with active record and retain idiomatic usage.

def update
  @whatever = Whatever.find(1) # selects the new values, undoing your lock
  @whatever.update_attributes params[:something]
end

Rails 1.2 will support pessimistic locking, which will add another
option to your toolbelt.

If you want to try it out,  have a look at optimistic_locking.rb in
activerecord,  it''s basically completely stand alone and your plugin
could do almost exactly the same thing.

-- 
Cheers

Koz

This issue has been addressed a number of times as patches in trac  
and emails to this mailing list.  A thorough search of both will turn  
them up.

--
Benjamin Curtis
http://www.bencurtis.com/
http://www.tesly.com/ -- Collaborative test case management
http://www.agilewebdevelopment.com/ -- Resources for the Rails community



On Jul 28, 2006, at 3:11 PM, Michael Genereux wrote:
> Given that even the most basic, multiuser apps hit concurrency  
> issues with editing different fields in the same record, I was  
> surprised that version locking was the only level of support Rails  
> offered.
>
> I would *love* to PDI field level change checking (I''ve done it  
> before on another framework) and also updating Base to only UPDATE  
> fields that have changed.  While it may sound funny, I''m a gung-ho
> person and would just need to be pointed in the direction of how to  
> make a plugin.  I wander through the ActiveRecord code all the time  
> but not all of Rails.  I was pondering adding support with a patch  
> and have some really great ideas but if it didn''t get accepted, I
> didn''t want to waste the time.  Also, how do I check to see if  
> someone else hasn''t already started a plugin/gem/patch for the
same
> issue?
>
> Bob Silva wrote:
>> I''m curious to know if "dirty" checking was
considered during the
>> design
>> process of AR. It would add performance benefits (configurable?)  
>> for larger
>> tables and associations in a write heavy environment especially if  
>> you have
>> text or blob fields in your tables. Anyone care to PDI this as a  
>> plugin?
>>
>> Bob Silva
>> http://i.nfectio.us/
>>
>>
>> -----Original Message-----
>> From: rails-core-bounces@lists.rubyonrails.org
>> [mailto:rails-core-bounces@lists.rubyonrails.org] On Behalf Of  
>> Caio Chassot
>> Sent: Thursday, July 27, 2006 8:03 AM
>> To: rails-core@lists.rubyonrails.org
>> Subject: Re: [Rails-core] Re: Why are has_one objects resaved when  
>> the
>> parentis saved?
>>
>>
>> On 2006-07-27, at 11:56 , Josh Susser wrote:
>>
>>> That''s a valid point.  But did you read
Jonathan''s patch?  It
>>> covers that case where you''re dealing with new records
just
>>> fine.   I think the issue he was correcting was that has_one  
>>> *always* saves  the child object, not just for new records.
>>>
>>
>> Strictly, it does not *always* save the child, only if it''s
been
>> *loaded*.
>>
>> It checks the association ivar directly, meaning it does not go   
>> through the accessor, and therefore does not try to load it.
>>
>> Saving loaded associations is a cheap way to make sure you save   
>> everytime a change *could have* happened. Without a dirtyness  
>> check,  I''d say that''s good enough.
>>
>> The alternative would be a handcoded dirtyness check, something like:
>>
>>    assoc_record.class.find(record.id).attributes.all? { |(k, v)|
>>      assoc_record.send(k) == v }
>>
>> Which quickly becomes expansive.
>>
>> _______________________________________________
>> Rails-core mailing list
>> Rails-core@lists.rubyonrails.org
>> http://lists.rubyonrails.org/mailman/listinfo/rails-core
>>
>> _______________________________________________
>> Rails-core mailing list
>> Rails-core@lists.rubyonrails.org
>> http://lists.rubyonrails.org/mailman/listinfo/rails-core
>>
>>
>
> -- 
> Michael Genereux
> SimianCodex - Web Application Development
> 949-874-6268
>
>
> _______________________________________________
> Rails-core mailing list
> Rails-core@lists.rubyonrails.org
> http://lists.rubyonrails.org/mailman/listinfo/rails-core


_______________________________________________
Rails-core mailing list
Rails-core@lists.rubyonrails.org
http://lists.rubyonrails.org/mailman/listinfo/rails-core

> If I am remembering correctly, maybe he could share what the issues were
> before someone dives into implementing a plugin for this.
Assuming you want to use dirty checking to write out a subset of the
columns for a given row, then the major reason to avoid it is data
integrity.  You have no guarantee that the data in your table will be
valid according to your business rules.   I seem to remember that this
was a big concern.
-- 
Cheers

Koz

On Jul 28, 2006, at 2:04 PM, Marcel Molina Jr. wrote:
> On Fri, Jul 28, 2006 at 01:07:59PM -0700, Bob Silva wrote:
>> I''m curious to know if "dirty" checking was
considered during the
>> design
>> process of AR. It would add performance benefits (configurable?)  
>> for larger
>> tables and associations in a write heavy environment especially if  
>> you have
>> text or blob fields in your tables. Anyone care to PDI this as a  
>> plugin?
>
> As I remember it, about a year ago Jeremy Kemper implemented dirty  
> column
> tracking  and decided against it for reasons I forget. Something  
> wasn''t right
> in the cosmos though.
The other way to do dirty checking is to do it on an object basis,  
instead of by columns.  I''d love to be able to track what associated  
objects had been modified so that when I save an order, all the line  
items that got modified get saved too.  Then of course I''ll want an  
identity map in AR so that I don''t have multiple, possibly  
inconsistent model objects floating around for just one record.  I  
think I can see a good way to do the identity map, but only the  
vaguest idea of how AR might save dirtied children automatically.   
Would these things be PDI-level enhancements, or a waste of time to  
look into?

--
Josh Susser
http://blog.hasmanythrough.com

On Jul 28, 2006, at 11:22 PM, Josh Susser wrote:
> The other way to do dirty checking is to do it on an object basis,  
> instead of by columns.  I''d love to be able to track what  
> associated objects had been modified so that when I save an order,  
> all the line items that got modified get saved too.  Then of course  
> I''ll want an identity map in AR so that I don''t have
multiple,
> possibly inconsistent model objects floating around for just one  
> record.  I think I can see a good way to do the identity map, but  
> only the vaguest idea of how AR might save dirtied children  
> automatically.  Would these things be PDI-level enhancements, or a  
> waste of time to look into?
I recall Jamis did object-level dirty checking by MD5ing attributes.  
Search for ''smart saves'' on trac (when it''s back with
us, the living-
breathing.)

jeremy

On 2006-07-29, at 03:22 , Josh Susser wrote:
> I think I can see a good way to do the identity map, but only the  
> vaguest idea of how AR might save dirtied children automatically.   
> Would these things be PDI-level enhancements, or a waste of time to  
> look into?
I think they stray too much from way we learned to expect AR to work.

Back in the rails 0.5 days I sorta remember this being discussed in  
the irc channel, and back then david didn''t feel like introducing the  
concept of dirtiness into the framework. In fact, the discussion was  
exactly regarding wether or not to save records when pushing into  
has_many assocs, and that''s what eventually generated the build and  
create methods.

Would be nice to see an implementation of dirtiness and identity map,  
but I''d be surprised if it ever got merged into the trunk.

On Jul 28, 2006, at 2:04 PM, Marcel Molina Jr. wrote:
> On Fri, Jul 28, 2006 at 01:07:59PM -0700, Bob Silva wrote:
>> I''m curious to know if "dirty" checking was
considered during the
>> design
>> process of AR. It would add performance benefits (configurable?)  
>> for larger
>> tables and associations in a write heavy environment especially if  
>> you have
>> text or blob fields in your tables. Anyone care to PDI this as a  
>> plugin?
>
> As I remember it, about a year ago Jeremy Kemper implemented dirty  
> column
> tracking  and decided against it for reasons I forget. Something  
> wasn''t right
> in the cosmos though.
>
> If I am remembering correctly, maybe he could share what the issues  
> were
> before someone dives into implementing a plugin for this.
Performance hurt. Having write_attribute mark dirty[attr] = true  
would be great, but attributes may be modified in-place once they''re  
read, so we have to hash or copy them and look for changes.

A seductive alternative to is to treat attribute values as immutable  
- freeze the value returned from read_attribute. Then we''re on easy  
street.

jeremy

Awesome.  I look forward to investigating.  I''d like to build something
that eventually leads to a coherent solution for people to choose 
between various locking and updating methodologies.

Jeremy Kemper wrote:
> On Jul 28, 2006, at 11:22 PM, Josh Susser wrote:
>> The other way to do dirty checking is to do it on an object basis, 
>> instead of by columns.  I''d love to be able to track what
associated
>> objects had been modified so that when I save an order, all the line 
>> items that got modified get saved too.  Then of course I''ll
want an
>> identity map in AR so that I don''t have multiple, possibly 
>> inconsistent model objects floating around for just one record.  I 
>> think I can see a good way to do the identity map, but only the 
>> vaguest idea of how AR might save dirtied children automatically.  
>> Would these things be PDI-level enhancements, or a waste of time to 
>> look into?
>
> I recall Jamis did object-level dirty checking by MD5ing attributes. 
> Search for ''smart saves'' on trac (when it''s back
with us, the
> living-breathing.)
>
> jeremy
> _______________________________________________
> Rails-core mailing list
> Rails-core@lists.rubyonrails.org
> http://lists.rubyonrails.org/mailman/listinfo/rails-core
>
-- 
Michael Genereux
SimianCodex - Web Application Development
949-874-6268

> Performance hurt. Having write_attribute mark dirty[attr] = true
> would be great, but attributes may be modified in-place once
they''re
> read, so we have to hash or copy them and look for changes.
>
> A seductive alternative to is to treat attribute values as immutable
> - freeze the value returned from read_attribute. Then we''re on
easy
> street.
It''s more than just dirty[attr] = true,  as most rails apps will be
calling attributes= or update_attributes which will flag as dirty,
every attribute in the form, irrespective of whether or not the value
changes.

The whole thing is high risk,  low reward.   Sounds perfect for a plugin! ;)



-- 
Cheers

Koz

I worked on a plugin called acts_as_modified a while ago, it works by
storing a copy of the original @attributes hash. I use this to be able
to send notification emails containing the changes, including the
original values, when objects are updated.

http://svn.viney.net.nz/things/rails/plugins/acts_as_modified

Seeing the discussion on this list I''ve put together acts_as_changed,
which stores a list of the changed attributes as they are modified. It
also only save attributes that have been changed. This plugin should
be more lightweight than acts_as_modified, but I haven''t done any
performance tests.

http://svn.viney.net.nz/things/rails/plugins/acts_as_changed

Both plugins freeze the attributes when they are read. I wasn''t able
to check out the existing patches because trac is down, so I''m not
sure how they compare with previous implementations.

Meanwhile, back on topic, can someone apply the original patch? :)

-Jonathan.

On 7/30/06, Michael Koziarski <michael@koziarski.com>
wrote:
> > Performance hurt. Having write_attribute mark dirty[attr] = true
> > would be great, but attributes may be modified in-place once
they''re
> > read, so we have to hash or copy them and look for changes.
> >
> > A seductive alternative to is to treat attribute values as immutable
> > - freeze the value returned from read_attribute. Then we''re
on easy
> > street.
>
> It''s more than just dirty[attr] = true,  as most rails apps will
be
> calling attributes= or update_attributes which will flag as dirty,
> every attribute in the form, irrespective of whether or not the value
> changes.
>
> The whole thing is high risk,  low reward.   Sounds perfect for a plugin!
;)
>
>
>
> --
> Cheers
>
> Koz
> _______________________________________________
> Rails-core mailing list
> Rails-core@lists.rubyonrails.org
> http://lists.rubyonrails.org/mailman/listinfo/rails-core
>