[CDBI] Updating a record - slight change proposal

William Ross will at spanner.org
Wed Oct 5 22:27:49 BST 2005


On 5 Oct 2005, at 17:04, Matt S Trout wrote:

> On Wed, Oct 05, 2005 at 04:14:24PM +0100, William Ross wrote:
>>
>> On 7 Sep 2005, at 10:33, William Ross wrote:
>>
>>
>> <snip>
>> 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?
>
>

In the general case the order of events goes something like this:

* call before_update triggers
* gather changed columns & values
* build and execute update
* call after_update triggers
* delete all changed columns (using previously gathered list)
* delete changed-column markers from $self->{__Changed}
* return 1

and as i understand it we are deleting changed columns in case  
after_update triggers or database functions change the values they  
contain. I suppose this is a good thing, though it's not something  
I've ever needed. The problem is that primary keys are not excluded  
from the list of fields that we discard, and a cdbi object missing  
some or all of its primary keys is no use to anyone.



> 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).
>
>

I think it's bug, not feature, and I'll suggest grepping out the  
primary key columns before discarding the changes.

I suppose you could argue that an object has no business appearing to  
change its primary keys and something is clearly wrong with the  
untainting of input in my useless rambling pile of an application,  
but I prefer the 'bug' explanation.

will






More information about the ClassDBI mailing list