Re: [CDBI] Updating a record - slight change proposal
[prev]
[thread]
[next]
[Date index for 2005/10/05]
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.
best
will
_______________________________________________
ClassDBI mailing list
ClassDBI@xxxxx.xxxxxxxxxxxxxxxx.xxx
http://lists.digitalcraftsmen.net/mailman/listinfo/classdbi