Steps to reproduce :
1- use a single field (any type, text will do)
2- edit a node, assign a value to the field, save the node
3- reedit the node, empty the value, save the node
The node keeps the value assigned in step 2

This is because we provide call drupal_write_record in 'update mode', with a record like 

array('nid' => x, 'vid' => y, 'field_foo' => NULL)

drupal_write_record overlooks the NULL entry and doesn't update field_foo.

Not sure if that's the intended behavior. Seems strange that drupal_write_record cannot be used to set a column to NULL. Moshe ?

CommentFileSizeAuthor
#5 d6-cck-null-drupal-write-record-3.patch5.71 KBdamien tournoud
#4 d6-cck-null-drupal-write-record-2.patch5.71 KBdamien tournoud
#3 d6-cck-null-drupal-write-record.patch5.67 KBdamien tournoud

Comments

yched’s picture

yched commented

Side note : this happens on non-multiple fields only, because updating a multiple field drops all existing records for the vid and uses d_w_r in 'insert' mode.

moshe weitzman’s picture

moshe weitzman commented

Yes, this is a bug which I had fixed but the fix was not accepted into D6. The fix has already been applied to D7. See http://drupal.org/node/181411. Hopefully someone will extract the right bits and post a patch for D6.

damien tournoud’s picture

damien tournoud commented
Status: Active » Needs review
StatusFileSize
new d6-cck-null-drupal-write-record.patch5.67 KB

Core drupal_write_record() does not allow NULL fields to be set. The missing part of Moshe's patch resolve a slightly different issue (saving empty serialized fields).

We could roll a patch for core drupal_write_record() to manage insertion and update of NULL fields, but in the meantime, here is a patch that implement a custom content_write_record().

damien tournoud’s picture

damien tournoud commented
StatusFileSize
new d6-cck-null-drupal-write-record-2.patch5.71 KB

Upon yched's comment, patch updated not to touch unset fields.

damien tournoud’s picture

damien tournoud commented
StatusFileSize
new d6-cck-null-drupal-write-record-3.patch5.71 KB

Oups, typo.

yched’s picture

yched commented
Status: Needs review » Fixed

I committed a slightly shorter version (without safety checks : if you try to set a 'not null' column to NULL, you get a SQL error - there are other similar cases). Thanks !

I need to turn this into a core patch. Not sure a fix in D6 core will cut it for us anyway. Do we want to go 'CCK requires D6.1' (when it exists) ?

moshe weitzman’s picture

moshe weitzman commented

6.1 will likely have security fixes in it so depending on that is quite reasonable. i'd rather not maintain our own private version of this function. sets a bad example for other contribs.

yched’s picture

yched commented

@moshe : yes, you're probably right. We'll see when it gets released :-) Not too keen on maintaining a separate version of this sensitive beast either.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.

zaczek’s picture

zaczek commented

This fix seems to produce another problem: http://drupal.org/node/486206

Cheers,
Jakub