Old databases might have a small {system}.weight
Damien Tournoud - October 11, 2009 - 18:38
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | base system |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed |
| Issue tags: | D7 upgrade path, Novice |
Description
Some databases that came from old Drupal installations seem to have a small {system}.weight. For example, on Drupal.org:
weight tinyint(3) unsignedThis breaks the upgrade process from Drupal 6 to Drupal 7 where we bump some modules to a weight > 1000.
#2
tagging
#3
.
#4
Ideally what should be done is to convert the 6.x system field values to match those of D7. What reason would there be on an upgrade not to do this. This will not break any 6.x sites doing this in case the update fails down stream.
Please review the following code, it does not use $schema as the table only need modification. I'm wonder though why D7 carries a int 11 in weights this seems a little high and would probably make more sense to use a small init 6 as in the schema_version field.
Index: includes/update.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/update.inc,v
retrieving revision 1.31
diff -u -p -r1.31 update.inc
--- includes/update.inc 25 Jan 2010 10:38:34 -0000 1.31
+++ includes/update.inc 27 Jan 2010 00:45:47 -0000
@@ -266,6 +266,19 @@ function update_fix_d7_requirements() {
file_put_contents(conf_path() . '/settings.php', "\n" . '$databases = ' . var_export($databases, TRUE) . ";\n\$drupal_hash_salt = '$salt';", FILE_APPEND);
}
if (drupal_get_installed_schema_version('system') < 7000 && !variable_get('update_d7_requirements', FALSE)) {
+ // Change 6.x system table field values to 7.x equivelent
+ db_change_field('system', 'status', 'status',
+ array('type' => 'int', 'length' => 11, 'not null' => TRUE, 'default' => 0));
+ db_change_field('system', 'bootstrap', 'bootstrap',
+ array('type' => 'int', 'length' => 11, 'not null' => TRUE, 'default' => 0));
+ db_change_field('system', 'weight', 'weight',
+ array('type' => 'int', 'length' => 11, 'not null' => TRUE, 'default' => 0));
+ db_change_field('system', 'schema_version', 'schema_version',
+ array('type' => 'int',
+ 'size' => 'small',
+ 'length' => 6,
+ 'not null' => TRUE,
+ 'default' => -1));
// Add the cache_path table.
$schema['cache_path'] = drupal_get_schema_unprocessed('system', 'cache');
$schema['cache_path']['description'] = 'Cache table used for path alias lookups.';
Patch attached based on yesterday's HEAD
#5
The last submitted patch, fixsystemweight.patch, failed testing.
#6
#612870: Weight fields should be int, not tinyint
#7
I see where there is similarities with #612870: Weight fields should be int, not tinyint and it has been committed. however, there is one table that still needs correction in addition to those, the system table, without fixing it there are some upgrades from d6 to d7 that will fail immediately. I've attached a corrected patch to fix it.
#8
Yes system was missing from the other patch.
+++ includes/update.inc 30 Jan 2010 19:30:48 -0000@@ -266,7 +266,33 @@ function update_fix_d7_requirements() {
+ // Change 6.x system table field values to 7.x equivelent
+ db_change_field('system', 'schema_version', 'schema_version',
...
+ 'default' => -1)
+ );
+ db_drop_index('system' ,'modules'); // D6 index
Should be "equivalent", also comments should be sentence case and include a period, and above the code they describe rather than inline on the same line - see http://drupal.org/coding-standards
Additionally, where we're dropping and recreating the same index - could we put those statements next to each other?
Looks like the path index comment appears twice in the patch?
Powered by Dreditor.
#9
Corrected patch as catch suggested. Fixed modules index where bootstrap field should not have been present.
#10
+++ includes/update.inc 31 Jan 2010 21:39:31 -0000@@ -266,7 +266,39 @@ function update_fix_d7_requirements() {
+
+ // Add the cache_path table.
+ $schema['cache_path'] = drupal_get_schema_unprocessed('system', 'cache');
+ $schema['cache_path']['description'] = 'Cache table used for path alias lookups.';
+ // Add the cache_path table.
$schema['cache_path'] = drupal_get_schema_unprocessed('system', 'cache');
$schema['cache_path']['description'] = 'Cache table used for path alias lookups.';
Duplicated code?
Powered by Dreditor.
#11
Hmmm, missed that one. Stupid copy and paste error a couple patches ago. My Bad
#12
Looks good now.
#13
subscribe
#14
This seems to have some comment indentation issues, and also warns me about fuzz when I apply it. I can haz re-roll?
#15
You can haz.
Also this really is critical - no Drupal 5 site can upgrade to Drupal 7 without this patch afaik.
#16
Awesome, thanks. Committed to HEAD!
#17
What's 'lenght' parameter means for INT data-type? Is this mysql specific? Suppose no!
Here is a patch to remove length from schema definition.
#18
Oops. :P
Thanks, committed to HEAD. :)
#19
Automatically closed -- issue fixed for 2 weeks with no activity.