Working on filefield.module I realized that content.module might work better if it knew whick fields needed to be deleted when updating multivalue fields...

I was having an issue where I would submit a file and errant values in node_field[$delta] would make content.module insert a new row in the field/content type table.

The existing approach is to just DELETE all the field data and reinsert for multivalues so any node_field[$delta] that is unset will disappear. I modified the code to delete if node_field[$delta] is null... So multivalue fields can set node_field[$delta] = NULL and content.module will be guaranteed to clean up.

To make a smaller set of code work more universally I also normalized how multivalue/single value fields were handled in the database... I decided to make an assumption that a delta column exists on all field tables... and work around that strategy. The net effect was the removal of a lot of conditional logic..
(I think there was some necessity to do this to make my targeted deletion stuff, but I lost it in a caffeine haze.)

the delta columns in the DB seem to behave well they reset themselves to 0 as necessary automagically.
I've tested with the stock field types, and things seem to be working properly... I can add content types and new fields without problem...

either way here is a patch to peruse.... you should only try it on fresh cck installs. I haven't written an update function yet. I wanted to bounce it off the other cck heads first.

CommentFileSizeAuthor
cck-multiple-normalize.patch.txt11.45 KBdopry

Comments

dopry’s picture

Status: Active » Needs review

that darn status thing...

killes@www.drop.org’s picture

I don't fully understand this patch yet, but I just want to add that I really dislike the delete/insert approach. It is also still existing in parts or Drupal core, but it should be replaced by a more sensible approach.

drewish’s picture

did some of this get applied to HEAD?

dopry’s picture

I think some of it applies to head, and I'm not sure if I wrote it cover shared and single field instance properly in retrospect... I'm pretty sure JonBob did some fixes to the delete stuff...

@JonBob should I go ahead and close this?

catch’s picture

Status: Needs review » Closed (fixed)

Assuming this got fixed by now, so closing directly due to age.