Re: [CDBI] Updating a record - slight change proposal

[prev] [thread] [next] [Date index for 2005/10/05]

On Wed, Oct 05, 2005 at 04:14:24PM +0100, William Ross wrote:
> 
> On 7 Sep 2005, at 10:33, William Ross wrote:
> 
> >On 7 Sep 2005, at 02:53, Kate Yoak wrote:
> >
> >>There is a slight problem here - the object you get back will be  
> >>entirely
> >>empty if $id is contained within %all_the_data.  The id field gets  
> >>discarded
> >>because it has been marked as changed.
> >>
> >>Here is a change I made in my cdbi class (I am overriding  
> >>_attribute_set):
> >>
> >> <snip>
> 
> >>for my $col (keys %$vals) {
> >>      #### ADDING  the unless statement here:
> >>      #$self->{__Changed}{$col}++ ;
> >>      $self->{__Changed}{$col}++ unless $self->_attr($col) eq  
> >>$vals->{$col};
> >>   }
> >>   $self->_attribute_store($vals);
> >>}
> >>
> >>In other words, don't mark something as changed if it hasn't  
> >>really changed.
> >>
> >>What did I break?
> >>
> >
> >The existing or new value will often be an object, which will often  
> >but not always stringify to the value that would be stored in the  
> >database, so testing with 'eq' isn't always going to work. You'd  
> >really have to use the same deflation mechanism as cdbi does when  
> >updating. I would imagine that Tony considered this before taking  
> >the easy route, but since he's not here you'd have to ask him  
> >directly.
> 
> 
> Hah! How fitting that two weeks after sending such a priggish  
> response to your question, I should be severely bitten by the same  
> bug. As a result I now think it's a Very Bad Thing and should be  
> Fixed :)
> 
> I still don't think the answer is to change the logic for is_changed,  
> but update() should definitely remove primary keys from its set of  
> changed columns before it discards the new values.
> 
> create() does exactly that, I now see. So I don't think it'll be that  
> controversial a change.

Bar MySQL stupidites, why do we need this at all? I mean, if your database
isn't able to store the values you've given it without messing with them
I would have thought it would throw an error on UPDATE?

The only situation I can see where we need to ask the DB for the column is in
the case of auto-inc PKs - I'm sure I'm missing an obvious problem here but
I thought I'd ask the floor (and I'm wondering whether this behaviour should
be core or optional, at least for DBIx::Class).

        -- 
             Matt S Trout       Specialists in perl consulting, web development, and
  Technical Director    UNIX/Linux systems architecture and automation. Mail
Shadowcat Systems Ltd.  mst (at) shadowcatsystems.co.uk for more information

 + Help us build a better perl ORM: http://dbix-class.shadowcatsystems.co.uk/ +