[CDBI] Updating a record - slight change proposal

William Ross will at spanner.org
Wed Oct 5 16:14:24 BST 2005

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.

I'll submit a bug as soon as I get time to write the patch and test  
that Tony will inevitably request.

Sorry about that: you were right all along, apart from details of the  
proposed fix.



More information about the ClassDBI mailing list