First pass at porting CCK to the new D6 Schema API :
- content_schema() now includes information about dynamic data tables.
- refactored content_admin to remove the now unneeded content_db_change_column() and content_db_add_column(). The monster content_alter_db_field() function is now much nicer.

For now, the 'old style' (MySQL-oriented) columns definitions for field modules coexist with the new 'Schema API' (db agnostic) ones. This is done by adding a temporary 'database columns schema' op for hook_field_settings (old style columns are still in the 'database columns' op). After a quick testing, the dynamic db operations seem to work well, so this temporary step could probably go soon.

Still TODO :
- see if content_admin.inc could be further simplified.
- remove the dual 'database columns / 'database columns schema' column definitions.
- maybe (er...) take the opportunity to split field CRUD cleanly between form handlers and API CRUD functions at last ?
- maybe (er...) take the opportunity to optimize load and insert operations (currently on query per field) ?

The patch also
- removes a call db_num_rows() (function is gone in D6) in noderef, which didn't seem to be needed anyway. There remain other occurrences in update functions, that would probably require a case-by-case look.
- removes some seemingly dead code in number_field_settings()

Attached is the patch I committed to HEAD.

CommentFileSizeAuthor
cck_27.patch28.84 KByched

Comments

moshe weitzman’s picture

moshe weitzman commented

subscribe ... the maybes would be quite nice :)

yched’s picture

yched commented

The previous commit did not actually make use of the new Schema-style column definitions for fields. It now does. Seems to work OK so far.

While testing this, I found (and hopefully fixed) a serious bug in our friend content_alter_db_field() (the function that updates the dynamic data tables layout) :
When a field update involves both changing the storage type (PER_FIELD / PER_CONTENT) and the number of columns, there are cases where the data migration simply fails and wipes all the data for the field.
Example : change a text field from single to multiple *and* from 'filtered' to 'plain text' : all your data is gone...

Text fields are probably the only ones whose storage columns can change depending on the fields settings ('format' column for 'filtered text'), and changing both 'multiple' and 'text processing' settings does not happen every day, so it's probably not *that* dramatic. Besides, that bug has probably been around since the early days of CCK, and we have not been flooded with bug reports for data loss, AFAIK.

I think this is fixed in HEAD now, but we might want to backport (and intensively test...) this for 5 and 4.7 branches...
I'll open a separate bug report and post a patch.

yched’s picture

yched commented

This seems to work right now, so I removed the previous 'old style' (MySQL-oriented) columns definitions completely.

I also merged the code branches for content_field()'s ops 'insert' and 'update', which were in fact the same.

yched’s picture

yched commented

The code in http://drupal.org/node/169982, whether ot gets committed to D6 core or not, should be an opener for the 'optimize load and insert operations' part.

matt_paz’s picture

matt_paz commented

subscribing

moshe weitzman’s picture

moshe weitzman commented
Status: Active » Fixed

i think this is done. if not, please reopen

Anonymous’s picture

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