| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | update system |
| Category: | bug report |
| Priority: | major |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | D7 upgrade path |
Issue Summary
Various facilities which were added as modules in Drupal 6 have been moved into core in D7 and consequently tables, fields and indexes they require are now created in core. During an update, some of these are created by update.inc. However, if the feature had been already been added to the D6 site as a module, some of these items may already exist. This leads to an SQL errors when update tries to create an item that already exists.
For example, if the D6 site has the Date module applied then the tables date_format_type, date_formats and date_format_locale already exist.
A search through update.inc reveals the following objects which are created without checking whether or not they already exist:
327 db_add_index('system', 'system_list'
329 // Add the cache_path table.
335-7 db_add_field('url_alias', 'source' & 'alias'
339-50 db_add_field('menu_router', 'delivery_callback' 'context' 'tab_parent' 'theme_callback' & 'theme_arguments'
352 // Add the role_permission table.
376 db_add_index('semaphore', 'value'
462 db_create_table('date_format_type'
463 db_create_table('date_formats'
464 db_create_table('date_format_locale'
Many of these cause SQL errors when updating the D6.16 site described in #785184: Attempts to update a D6.16 site to D7 with CCK, imagecache, imagefield, date, etc applied.
The attached patch checks for or drops all of these items before attempting to create them.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| Update_drop_database_objects_before_create.patch | 3.53 KB | Idle | PASSED: [[SimpleTest]]: [MySQL] 20,359 pass(es). | View details |
Comments
#1
Tagging for upgrade path
#2
Everything except for date formats should not be in an unhacked D6 site.
Date formats we should deal with though.
#3
Somehow overlooked that there's a patch here. I'd not be overly opposed to adding this check in general - cache_path may be added in pressflow, the issue is currently against D6 although I doubt it'll get applied. If we do that though, it should use db_field_exists() for the columns instead of dropping and recreating. We also have http://api.drupal.org/api/function/db_index_exists/7.
#4
Just wanted to add a reference to #735000: Installing image.module on a upgrades site does not install schema because of module name overlap, where we already added some special handling for image.module
#5
only D6 sites with the date module are effected?
In preparation for the new "major" issue priority, I'm tagging this (not actually critical, but still important) issue as priority-major.
See: http://drupal.org/node/669048#comment-3019824 (#669048: We should have four priorities not three (introduce a new 'major' priority)) for more information about the coming major issue priority.
#6
I have done some more investigating and tracked the changes made to D6 by various modules and, yes, it seems that the three tables added by the Date module are the main problem here. I looked at the stuff suggested by Berdir in #4 and I am not sure this approach quite works the same here because update.inc attempts to create the D7 tables (which is where the error occurs) quite early in the process and before any of the module update code is called.
There are three tables involved: tables date_format_type, date_formats and date_format_locale. Their fields have the same names in D6 and D7 but the data type is slightly different in some cases and the collation settings are different. As far as I can see, the data they contain in D6, at least for UK locale date settings, looks the same as the D7 version.
To adopt the approach Bedir suggests, and to migrate the D6 settings to D7, I think that update.inc would need to check for and rename the D6 tables so they didn't clash with the D7 ones it creates, and then the Date module update code would need to check for the existence of the renamed, old tables, copy data across from them and finally drop them.
#7
Change title.
#8
That sounds sane for an upgrade path.
#9
Actually in D6 Date, the table is called 'date_format_types' and in D7 'date_format_type' so we don't need to migrate, that. We could, though, for sake of consistency. Anyway, it would be good to ping KarenS before this goes in, so she knows of the plan.
#10
This simple patch worked for me. I don't have any cool upgrade scripts, so I only tested it with the patch (it works!) and not without, but I'm going to believe common sense and the people who reported here, that that won't go well.
I have the old d6_tables in my DB after the upgrade, together with the new D7 ones.
Just for reference: I didn't have any data in them, but I don't think that should make a difference.
#11
Looks fine to me.
Opened #820670: date format schema changes / upgrade path.
#12
The last submitted patch, 799982-date-upgrade.patch, failed testing.
#13
#10: 799982-date-upgrade.patch queued for re-testing.
#14
#15
I think I'd like to run this past KarenS first, but it looks basically fine to me, and consistent with how we handled the D5 => D6 Actions upgrade path.
#16
I haven't had time to test this, but the approach looks fine to me. If I get a chance I'll come back and test, but it doesn't look like it could hurt anything.
#17
Ok, great. Committed to HEAD!
#18
Automatically closed -- issue fixed for 2 weeks with no activity.
#19