Rails core - Apr 2009 - Built associations don't know their parent. Why not?

(Just posted it in Talk and realized it makes more sense to ask here.)

Simple example.

class Car < ActiveRecord::Base
  has_one :engine

  validates_presence_of :engine
  validates_associated :engine
end

class Engine < ActiveRecord::Base
  belongs_to :car

  validates_presence_of :car
end

@car = Car.new
@car.valid? # => false
@car.errors # => no engine obviously

@car.build_engine
@car.valid? # => false

How come??

@car.errors # => engine has no car!
@car.engine.car # => nil

Since we validate_associated :engine, engine runs its own validations
and finds that  validates_presence_of car is false, since car is nil.
Shouldn''t engine know the car that built it?

Just for fun I do this:

@car.engine.car = @car
@car.valid? # => true

Shouldn''t this work by default?

I agree. Please write a patch :)

Eloy

On 22 apr 2009, at 18:18, hakunin wrote:
>
> (Just posted it in Talk and realized it makes more sense to ask here.)
>
> Simple example.
>
> class Car < ActiveRecord::Base
>  has_one :engine
>
>  validates_presence_of :engine
>  validates_associated :engine
> end
>
> class Engine < ActiveRecord::Base
>  belongs_to :car
>
>  validates_presence_of :car
> end
>
> @car = Car.new
> @car.valid? # => false
> @car.errors # => no engine obviously
>
> @car.build_engine
> @car.valid? # => false
>
> How come??
>
> @car.errors # => engine has no car!
> @car.engine.car # => nil
>
> Since we validate_associated :engine, engine runs its own validations
> and finds that  validates_presence_of car is false, since car is nil.
> Shouldn''t engine know the car that built it?
>
> Just for fun I do this:
>
> @car.engine.car = @car
> @car.valid? # => true
>
> Shouldn''t this work by default?
> >

Shouldn''t you be validating presence of the key rather than the object
itself?

On Apr 22, 2009, at 2:29 PM, Eloy Duran wrote:
>
> I agree. Please write a patch :)
>
> Eloy
>
> On 22 apr 2009, at 18:18, hakunin wrote:
>
>>
>> (Just posted it in Talk and realized it makes more sense to ask  
>> here.)
>>
>> Simple example.
>>
>> class Car < ActiveRecord::Base
>> has_one :engine
>>
>> validates_presence_of :engine
>> validates_associated :engine
>> end
>>
>> class Engine < ActiveRecord::Base
>> belongs_to :car
>>
>> validates_presence_of :car
>> end
>>
>> @car = Car.new
>> @car.valid? # => false
>> @car.errors # => no engine obviously
>>
>> @car.build_engine
>> @car.valid? # => false
>>
>> How come??
>>
>> @car.errors # => engine has no car!
>> @car.engine.car # => nil
>>
>> Since we validate_associated :engine, engine runs its own validations
>> and finds that  validates_presence_of car is false, since car is nil.
>> Shouldn''t engine know the car that built it?
>>
>> Just for fun I do this:
>>
>> @car.engine.car = @car
>> @car.valid? # => true
>>
>> Shouldn''t this work by default?
>>>
>
>
> >

A key wouldn''t be possible because you''d need to save a
possibly
invalid record first to be able to do that.
Let me note that I don''t necessarily have a need for  
validate_presence_of to work in this case, but rather the ability of  
having the parent object around when doing stuff in the child model.

Eloy

On 22 apr 2009, at 20:52, Flinn Mueller wrote:
>
> Shouldn''t you be validating presence of the key rather than the
object
> itself?
>
> On Apr 22, 2009, at 2:29 PM, Eloy Duran wrote:
>
>>
>> I agree. Please write a patch :)
>>
>> Eloy
>>
>> On 22 apr 2009, at 18:18, hakunin wrote:
>>
>>>
>>> (Just posted it in Talk and realized it makes more sense to ask
>>> here.)
>>>
>>> Simple example.
>>>
>>> class Car < ActiveRecord::Base
>>> has_one :engine
>>>
>>> validates_presence_of :engine
>>> validates_associated :engine
>>> end
>>>
>>> class Engine < ActiveRecord::Base
>>> belongs_to :car
>>>
>>> validates_presence_of :car
>>> end
>>>
>>> @car = Car.new
>>> @car.valid? # => false
>>> @car.errors # => no engine obviously
>>>
>>> @car.build_engine
>>> @car.valid? # => false
>>>
>>> How come??
>>>
>>> @car.errors # => engine has no car!
>>> @car.engine.car # => nil
>>>
>>> Since we validate_associated :engine, engine runs its own  
>>> validations
>>> and finds that  validates_presence_of car is false, since car is  
>>> nil.
>>> Shouldn''t engine know the car that built it?
>>>
>>> Just for fun I do this:
>>>
>>> @car.engine.car = @car
>>> @car.valid? # => true
>>>
>>> Shouldn''t this work by default?
>>>>
>>
>>
>>>
>
>
> >

Eloy, I''ll give it a shot. : )

Flinn, not necessarily. In this case we talk about associated in-
memory object which doesn''t have id yet. In my opinion
validates_presence_of :engine_id should behave the old way - check for
id - since we explicitly ask for it.


On Apr 22, 2:52 pm, Flinn Mueller <theflinns...@gmail.com>
wrote:
> Shouldn''t you be validating presence of the key rather than the
object  
> itself?
>
> On Apr 22, 2009, at 2:29 PM, Eloy Duran wrote:
>
>
>
> > I agree. Please write a patch :)
>
> > Eloy
>
> > On 22 apr 2009, at 18:18, hakunin wrote:
>
> >> (Just posted it in Talk and realized it makes more sense to ask  
> >> here.)
>
> >> Simple example.
>
> >> class Car < ActiveRecord::Base
> >> has_one :engine
>
> >> validates_presence_of :engine
> >> validates_associated :engine
> >> end
>
> >> class Engine < ActiveRecord::Base
> >> belongs_to :car
>
> >> validates_presence_of :car
> >> end
>
> >> @car = Car.new
> >> @car.valid? # => false
> >> @car.errors # => no engine obviously
>
> >> @car.build_engine
> >> @car.valid? # => false
>
> >> How come??
>
> >> @car.errors # => engine has no car!
> >> @car.engine.car # => nil
>
> >> Since we validate_associated :engine, engine runs its own
validations
> >> and finds that  validates_presence_of car is false, since car is
nil.
> >> Shouldn''t engine know the car that built it?
>
> >> Just for fun I do this:
>
> >> @car.engine.car = @car
> >> @car.valid? # => true
>
> >> Shouldn''t this work by default?
>
>

Well, if the child cannot exist without parent, it seems natural to
have such validation in the child - must have parent. (In addition to
your reasoning.)

On Apr 22, 3:26 pm, Eloy Duran <eloy.de.en...@gmail.com>
wrote:
> A key wouldn''t be possible because you''d need to save a
possibly  
> invalid record first to be able to do that.
> Let me note that I don''t necessarily have a need for  
> validate_presence_of to work in this case, but rather the ability of  
> having the parent object around when doing stuff in the child model.
>
> Eloy
>
> On 22 apr 2009, at 20:52, Flinn Mueller wrote:
>
>
>
> > Shouldn''t you be validating presence of the key rather than
the object
> > itself?
>
> > On Apr 22, 2009, at 2:29 PM, Eloy Duran wrote:
>
> >> I agree. Please write a patch :)
>
> >> Eloy
>
> >> On 22 apr 2009, at 18:18, hakunin wrote:
>
> >>> (Just posted it in Talk and realized it makes more sense to
ask
> >>> here.)
>
> >>> Simple example.
>
> >>> class Car < ActiveRecord::Base
> >>> has_one :engine
>
> >>> validates_presence_of :engine
> >>> validates_associated :engine
> >>> end
>
> >>> class Engine < ActiveRecord::Base
> >>> belongs_to :car
>
> >>> validates_presence_of :car
> >>> end
>
> >>> @car = Car.new
> >>> @car.valid? # => false
> >>> @car.errors # => no engine obviously
>
> >>> @car.build_engine
> >>> @car.valid? # => false
>
> >>> How come??
>
> >>> @car.errors # => engine has no car!
> >>> @car.engine.car # => nil
>
> >>> Since we validate_associated :engine, engine runs its own  
> >>> validations
> >>> and finds that  validates_presence_of car is false, since car
is  
> >>> nil.
> >>> Shouldn''t engine know the car that built it?
>
> >>> Just for fun I do this:
>
> >>> @car.engine.car = @car
> >>> @car.valid? # => true
>
> >>> Shouldn''t this work by default?
>
>

I had a closely related question today, which was why this code does N
+1 queries?

order.order_items.each {|oi| oi.order}

Why must each order item do a query to find its order?  It seems like
Rails should recognize the reciprocal nature of the associations.


-kevin



On Apr 22, 9:18 am, hakunin <m...@bitsonnet.com>
wrote:
> (Just posted it in Talk and realized it makes more sense to ask here.)
>
> Simple example.
>
> class Car < ActiveRecord::Base
>   has_one :engine
>
>   validates_presence_of :engine
>   validates_associated :engine
> end
>
> class Engine < ActiveRecord::Base
>   belongs_to :car
>
>   validates_presence_of :car
> end
>
> @car = Car.new
> @car.valid? # => false
> @car.errors # => no engine obviously
>
> @car.build_engine
> @car.valid? # => false
>
> How come??
>
> @car.errors # => engine has no car!
> @car.engine.car # => nil
>
> Since we validate_associated :engine, engine runs its own validations
> and finds that  validates_presence_of car is false, since car is nil.
> Shouldn''t engine know the car that built it?
>
> Just for fun I do this:
>
> @car.engine.car = @car
> @car.valid? # => true
>
> Shouldn''t this work by default?

On Thu, Apr 23, 2009 at 9:16 AM, kscaldef <kevin.scaldeferri@gmail.com>
wrote:
>
> I had a closely related question today, which was why this code does N
> +1 queries?
>
> order.order_items.each {|oi| oi.order}
>
> Why must each order item do a query to find its order?  It seems like
> Rails should recognize the reciprocal nature of the associations.
Yes,  this is called bi-directional associations and would be a
welcome (if non-trivial) addition in 3.0.

Incidentally it actually only does 1 query, the rest are cache hits,
so at least your database is safe.  However it still wastes memory and
can lead to some weirdness and should be fixed.

The has_many :foos, belongs_to :bar case is very easy, but there are
some common corner cases which we''ll have to look at.  For example
what''s the inverse of belongs_to :bar when we have the following
associations to choose from:

has_many :foos, :conditions=>...
has_many :something_else, :class_name=>"Foo"
has_many :important_foos, :through=>...

Ideally we''d just punt on those cases and require users to explicitly
mark associations as inverses with a declaration like

has_many :foos, :conditions ... :inverse_of=>:bar

-- 
Cheers

Koz

On Wed, Apr 22, 2009 at 3:42 PM, Michael Koziarski
<michael@koziarski.com> wrote:
>
> On Thu, Apr 23, 2009 at 9:16 AM, kscaldef
<kevin.scaldeferri@gmail.com> wrote:
>>
>> I had a closely related question today, which was why this code does N
>> +1 queries?
>>
>> order.order_items.each {|oi| oi.order}
>>
>> Why must each order item do a query to find its order?  It seems like
>> Rails should recognize the reciprocal nature of the associations.
>
> Yes,  this is called bi-directional associations and would be a
> welcome (if non-trivial) addition in 3.0.
>
> Incidentally it actually only does 1 query, the rest are cache hits,
> so at least your database is safe.  However it still wastes memory and
> can lead to some weirdness and should be fixed.
>
> The has_many :foos, belongs_to :bar case is very easy, but there are
> some common corner cases which we''ll have to look at.  For example
> what''s the inverse of belongs_to :bar when we have the following
> associations to choose from:
>
> has_many :foos, :conditions=>...
> has_many :something_else, :class_name=>"Foo"
> has_many :important_foos, :through=>...
>
> Ideally we''d just punt on those cases and require users to
explicitly
> mark associations as inverses with a declaration like
>
> has_many :foos, :conditions ... :inverse_of=>:bar
FWIW, that''s basically how Sequel handles it, with the :reciprocal
option, though instead of punting by default if multiple associations
match, it picks the first matching association.

Jeremy

> FWIW, that''s basically how Sequel handles it, with the :reciprocal
> option, though instead of punting by default if multiple associations
> match, it picks the first matching association.
The first matching association sounds like it could be prone to
surprises but we could see how it plays out.  Fundamentally though
having something that works with explicit declarations only would be a
good start.  How we mark the associations is kinda orthogonal to the
associations'' behaviour which is the real challenge.


-- 
Cheers

Koz

Wouldn''t all of these problems be solved by an identity map?
Wouldn''t that
be a better solution than trying to hack this on the existing system?

/Jonas


On Thu, Apr 23, 2009 at 1:26 AM, Michael Koziarski
<michael@koziarski.com>wrote:
>
> > FWIW, that''s basically how Sequel handles it, with the
:reciprocal
> > option, though instead of punting by default if multiple associations
> > match, it picks the first matching association.
>
> The first matching association sounds like it could be prone to
> surprises but we could see how it plays out.  Fundamentally though
> having something that works with explicit declarations only would be a
> good start.  How we mark the associations is kinda orthogonal to the
> associations'' behaviour which is the real challenge.
>
>
> --
> Cheers
>
> Koz
>
> >
>
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

2009/4/22 Michael Koziarski <michael@koziarski.com>
>
> On Thu, Apr 23, 2009 at 9:16 AM, kscaldef
<kevin.scaldeferri@gmail.com>
> wrote:
> >
> > I had a closely related question today, which was why this code does N
> > +1 queries?
> >
> > order.order_items.each {|oi| oi.order}
> >
> > Why must each order item do a query to find its order?  It seems like
> > Rails should recognize the reciprocal nature of the associations.
>
> Yes,  this is called bi-directional associations and would be a
> welcome (if non-trivial) addition in 3.0.
>
> Incidentally it actually only does 1 query, the rest are cache hits,
> so at least your database is safe.  However it still wastes memory and
> can lead to some weirdness and should be fixed.
>
> The has_many :foos, belongs_to :bar case is very easy, but there are
> some common corner cases which we''ll have to look at.  For example
> what''s the inverse of belongs_to :bar when we have the following
> associations to choose from:
>
> has_many :foos, :conditions=>...
> has_many :something_else, :class_name=>"Foo"
> has_many :important_foos, :through=>...
>
> Ideally we''d just punt on those cases and require users to
explicitly
> mark associations as inverses with a declaration like
>
> has_many :foos, :conditions ... :inverse_of=>:bar
>
I wrote this up as a patch a while ago, not enough folk looked at it from
Rails 2.3, but maybe more people can look at it for 3.0? The patch is here:
http://rails.lighthouseapp.com/projects/8994/tickets/1619-patch-support-for-inverse-option-in-associations

It''s based on a plugin I wrote called parental_control :
http://github.com/hlame/parental_control

The patch demands a :inverse syntax, but the plugin uses "magic" to
try and
work it out correctly.

Cheers,

Muz

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

2009/4/24 Murray Steele <murray.steele@gmail.com>
> 2009/4/22 Michael Koziarski <michael@koziarski.com>
>
>>
>> On Thu, Apr 23, 2009 at 9:16 AM, kscaldef
<kevin.scaldeferri@gmail.com>
>> wrote:
>> >
>> > I had a closely related question today, which was why this code
does N
>> > +1 queries?
>> >
>> > order.order_items.each {|oi| oi.order}
>> >
>> > Why must each order item do a query to find its order?  It seems
like
>> > Rails should recognize the reciprocal nature of the associations.
>>
>> Yes,  this is called bi-directional associations and would be a
>> welcome (if non-trivial) addition in 3.0.
>>
>> Incidentally it actually only does 1 query, the rest are cache hits,
>> so at least your database is safe.  However it still wastes memory and
>> can lead to some weirdness and should be fixed.
>>
>> The has_many :foos, belongs_to :bar case is very easy, but there are
>> some common corner cases which we''ll have to look at.  For
example
>> what''s the inverse of belongs_to :bar when we have the
following
>> associations to choose from:
>>
>> has_many :foos, :conditions=>...
>> has_many :something_else, :class_name=>"Foo"
>> has_many :important_foos, :through=>...
>>
>> Ideally we''d just punt on those cases and require users to
explicitly
>> mark associations as inverses with a declaration like
>>
>> has_many :foos, :conditions ... :inverse_of=>:bar
>>
>
> I wrote this up as a patch a while ago, not enough folk looked at it from
> Rails 2.3, but maybe more people can look at it for 3.0? The patch is here:
>
http://rails.lighthouseapp.com/projects/8994/tickets/1619-patch-support-for-inverse-option-in-associations
>
> It''s based on a plugin I wrote called parental_control :
> http://github.com/hlame/parental_control
>
Um ... that''s http://github.com/h-lame/parental_control

I keep forgetting which apps allow / don''t allow hyphens in
usernames...

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Thu, Apr 23, 2009 at 6:24 PM, Jonas Nicklas <jonas.nicklas@gmail.com>
wrote:
> Wouldn''t all of these problems be solved by an identity map?
Wouldn''t that
> be a better solution than trying to hack this on the existing system?
Yes, bi-directional associations are a localised implementation of an
identity map.  As has been discussed several times before, an identity
map would be a welcome addition to AR, but would be a *lot* of work
given the functionality we support (:select etc) and the strange ways
it can all interact.

Bi-Directional associations would give 99% of the return with much
less of the complication.

-- 
Cheers

Koz

On Thu, Apr 23, 2009 at 6:24 PM, Jonas Nicklas <jonas.nicklas@gmail.com>
wrote:
> Wouldn''t all of these problems be solved by an identity map?
Wouldn''t that
> be a better solution than trying to hack this on the existing system?
Correct me if I''m wrong, but an identity map does nothing to help with
unsaved records.  For example, if I build children on a parent record,
I should be able to have validates_presence_of :parent in the child
model - but can''t.

Of course, this already works fine without an identity map in the
update case, where the parent ID has already been set and so the
record can be found, but create is the hard one.  So an identity map
doesn''t really solve many problems, though there are potentially
efficiency gains.

I wrote a "reverse" association finder in ActiveScaffold a couple
years ago. Works pretty well, I think:

http://github.com/activescaffold/active_scaffold/blob/1252ee785d5d3fff4fd158821cc55ae6127c291c/lib/extensions/reverse_associations.rb

Also, I''ve grown very accustomed to writing bi-directional association
validations in my home-grown validation library. Granted, I''ve always
had to set the "reverse" association manually which looks a little
shoddy in my controllers, but the key to making it all work was:

http://github.com/cainlevy/semantic-attributes/blob/6103e695ef47b625bdc81634dbc1d914cf28b6ff/lib/active_record/validation_recursion_control.rb


On Apr 25, 2:52 am, Will Bryant <will.bry...@gmail.com>
wrote:
> On Thu, Apr 23, 2009 at 6:24 PM, Jonas Nicklas
<jonas.nick...@gmail.com> wrote:
> > Wouldn''t all of these problems be solved by an identity map?
Wouldn''t that
> > be a better solution than trying to hack this on the existing system?
>
> Correct me if I''m wrong, but an identity map does nothing to help
with
> unsaved records.  For example, if I build children on a parent record,
> I should be able to have validates_presence_of :parent in the child
> model - but can''t.
>
> Of course, this already works fine without an identity map in the
> update case, where the parent ID has already been set and so the
> record can be found, but create is the hard one.  So an identity map
> doesn''t really solve many problems, though there are potentially
> efficiency gains.