[Dbix-class] Bug with relationships on new objects.

[Matt S Trout]

On Mon, Feb 18, 2008 at 10:01:16AM +1100, brett gardner wrote:
> Matt S Trout wrote:
> >On Fri, Jan 25, 2008 at 12:41:55PM +1100, brett gardner wrote:
> >  
> >>Matt S Trout wrote:
> >>    
> >>>Which is, as I said, nothing to do with new vs. inflate_result.
> >>>
> >>>If you pass 'col => undef' to new(), you get the fk = NULL.
> >>>
> >>>If you use the 'columns' resultset attr to restrict the col set to not
> >>>include the column, you get the 'column does not exist'[0] behaviour.
> >>>
> >>>But all this is academic; if !has_column_loaded we should just be 
> >>>throwing
> >>>an exception, not creating the resultset anyway.
> >>>
> >>>Who fancies having a go at a patch? :)
> >>>
> >>>[0] which you call 'column is not set', which is almost accurate but not
> >>>quite :)
> >>>
> >>> 
> >>>      
> >>Right, well I wrote a patch (attached) that fixes the problem, but then 
> >>    
> >
> >Your patch seems to single out only the primary key, so you only fix
> >half the problem.
> >
> >Why are you restricting it to this when any non-existant key column can
> >cause the exact same bug?
> >  
> If you always throw an exception this could cause problems with the 
> following scenario.
> 
> Say you have "Agents", some are represented by an "Agency" and some are 
> "free agents".
> 
> So the "Agent" has a belongs to relationship to an "Agency" but it is 
> not required eg the field "agency_id" in the table "agents" can be set 
> to null.
> 
> If the behaviour was changed to always throw an exception doing the 
> following would not work.
> 
>    if ($agent->agency){
>        print "Belongs to agency";     
>    }else{
>        print "Free agent";
>    }

Then there's a bug in your patch. Fix that, don't just hack round it by
disabling your changes for important keys.

Actually, I'm not sure that would even fail - if the key's undef, DBIC
generally doesn't inflate the related object ...

-- 
      Matt S Trout       Need help with your Catalyst or DBIx::Class project?
   Technical Director                    http://www.shadowcat.co.uk/catalyst/
 Shadowcat Systems Ltd.  Want a managed development or deployment platform?
http://chainsawblues.vox.com/            http://www.shadowcat.co.uk/servers/