drop the sequences and queue tables from D5 during the D6 -> D7 upgrade [#719730]

In trying to upgrade http://anarcat.koumbit.org/ to D7 a bit naively, I got a blank page after a long time of loading in update.php. The apache error log tells me:

[Fri Feb 19 14:49:31 2010] [error] [client 72.0.207.41] PHP Fatal error:  Call to undefined function _system_rebuild_theme_data() in /srv/aegir/drupal-7.x-2010219/includes/theme.inc on line 588

This is with this morning's dev snapshot. I installed my blog using the following procedure:

drush dl drupal-7.x
mv drupal-7.x-dev drupal-7.x-2010219
cd drupal-7.x-2010219/sites
tar zfx ~/backups/anarcat.koumbit.org-20100219.141805.tar.gz ./database.sql
cp -Rp default anarcat.test.koumbit.net
cd anarcat.test.koumbit.net
cp default.settings.php settings.php
vim settings.php # setup $database string
mysql -u anarcat_d7 -p -h mysql.koumbit.net anarcat_d7 < ~/database.sql 
cp anarcat.koumbit.org_80 anarcat.test.koumbit.net_80
vim anarcat.test.koumbit.net_80
sudo apache2ctl graceful

I can also reproduce on current head.

Arguably, the issue may be that my whole sites/ -specific modules are just missing (since they are not ported to D7 yet). Specifically, I'm using the Barlow theme, which was not ported to D7...

Comment	File	Size	Author
#20	719730_4.patch	1.21 KB	peterx
#18	719730_3.patch	1.25 KB	ctmattice1
#10	719730.patch	1.19 KB	anarcat
#9	719730.patch	1.36 KB	anarcat
#6	719730.patch	628 bytes	anarcat
#4	719730.patch	826 bytes	anarcat

Comments

Comment #1

anarcat CreditAttribution: anarcat commented 19 February 2010 at 20:29

Title:	update.php crashes with a white page + Call to undefined function _system_rebuild_theme_data()	» update.php crashes with a white page
Assigned:	Unassigned	» anarcat

Here's a crazy backtrace of the problem, right before the error

First, this probably means that #325169: Move error/exception handler higher up in the bootstrap process wasn't completely fixed. Second, this looks like the upgrade was attempted once, failed and tried again. Indeed, the first time I ran the upgrade, I made a typo in my database settings that yielded the awesome warning:

[Fri Feb 19 14:38:32 2010] [error] [client 72.0.207.41] PHP Fatal error:  Database::require_once(): Failed opening required '/srv/aegir/drupal-7.x-2010219/includes/database/myqsl/database.inc' (include_path='.:/usr/share/php:/usr/share/pear') in /srv/aegir/drupal-7.x-2010219/includes/database/database.inc on line 1506

notice the "myqsl" in the path (i really meant "mysql", i swear ;). That warning also led to a blank page, whichi #325169: Move error/exception handler higher up in the bootstrap process was supposed to prevent.

Since I may be misinterpreting this whole thing, I'm not marking this as a dupe but I'm changing the title to something more generic. I'll do more testing now.

Comment #2

anarcat CreditAttribution: anarcat commented 19 February 2010 at 20:33

Issue tags:

+D7csmtl

tagging this for the code sprint karma.

Comment #3

anarcat CreditAttribution: anarcat commented 19 February 2010 at 20:52

So clearly, there's an issue here. I reseted the database to my D6 dump and ran update.php again, and got this:

Of course followed by the theme error:

[Fri Feb 19 15:47:27 2010] [error] [client 72.0.207.41] PHP Fatal error:  Call to undefined function _system_rebuild_theme_data() in /srv/aegir/drupal-7.x-git/includes/theme.inc on line 589

Comment #4

anarcat CreditAttribution: anarcat commented 19 February 2010 at 21:34

Title:	update.php crashes with a white page	» update.php crashes with a white page on sequences
Status:	Active	» Needs review

File	Size
719730.patch	826 bytes

So the originating issue here is with the sequences table. It seems that even though I have a Drupal 6 database, it has a sequences table (!!):

mysql> describe sequences;
+-------+------------------+------+-----+---------+-------+
| Field | Type             | Null | Key | Default | Extra |
+-------+------------------+------+-----+---------+-------+
| name  | varchar(255)     | NO   | PRI |         |       | 
| id    | int(10) unsigned | NO   |     | 0       |       | 
+-------+------------------+------+-----+---------+-------+
2 rows in set (0.00 sec)

So I don't know: maybe my D5-D6 upgrade failed or partially failed at some point?

I have made the attached patch to make sure the table doesn't exist before we go around creating it.

Comment #5

scor CreditAttribution: scor commented 19 February 2010 at 21:36

+++ includes/theme.inc
@@ -585,6 +585,7 @@ function list_themes($refresh = FALSE) {
+      print_r(debug_backtrace());

not sure we need that

Comment #6

anarcat CreditAttribution: anarcat commented 19 February 2010 at 21:44

File	Size
719730.patch	628 bytes

The previous patch would introduce a regression for properly installed D6, this one checks if the table really exists before dropping it (something db_drop_table() should do IMHO).

It also removes a debugging statement.

Comment #7

scor CreditAttribution: scor commented 19 February 2010 at 21:49

+++ includes/update.inc
@@ -542,6 +542,11 @@ function update_fix_d7_requirements() {
+    // some Drupal 6 installs still have an unused sequences tables if they
+    // were upgraded from D5, ditch it

ucfirst($this);
$this . '.';

Is it safe to remove this table? Maybe we should first check where it came from. Are there contrib modules using it?

Comment #8

anarcat CreditAttribution: anarcat commented 19 February 2010 at 22:04

Title:

update.php crashes with a white page on sequences

» update.php crashes with a white page on sequences on drupal 6 sites upgraded from drupal 5

Well, Drupal 6 *doesn't* have the table and doesn't use it. If contrib modules use it, it's a bug.

Comment #9

anarcat CreditAttribution: anarcat commented 19 February 2010 at 22:15

File	Size
719730.patch	1.36 KB

Ugh. So I just caught up with #563106: Cannot upgrade from Drupal 6 to Drupal 7 - meta issue and there are a few comments in there regarding this issue, and a few patches. It's getting overly complicated there, so I suggest that specific issue is moved here separately.

So okay, I followed the drift in the other issue of renaming the table instead of completely dropping it. I think we should eventually ditch it, but I guess that will be part of the D8 release now. :P

Comment #10

anarcat CreditAttribution: anarcat commented 19 February 2010 at 23:05

Title:

update.php crashes with a white page on sequences on drupal 6 sites upgraded from drupal 5

» drop the sequences and queue tables from D5 during the D6 -> D7 upgrade

File	Size
719730.patch	1.19 KB

So I made up my mind about this. The sequences table (and I assume it's the same for queue) were *deprecated* in D6. They are not supported anymore, and a clean install of D6 will not have such tables. So any D6 contrib module relying on those tables is broken and was not properly upgraded. If a contrib module ported to D7 *still* uses the table, it deserves what it gets (that is, SQL errors).

The proper upgrade paths for those modules is finish the D6 upgrade process and stop using the sequences table in a last D6 releasee before the jump to D7.

Anyways, if a module uses the sequences table in D6, it's going to crap out on D7 because the structure is totally different, it will just not work.

So my vote goes to kill the table, and here's a patch that does that.

The reasoning applies to the sequences table, which I am more familiar with, but I extend it to the queues table, based on the textual patch provided here: http://drupal.org/node/563106#comment-2564942

Comment #11

peterx CreditAttribution: peterx commented 19 February 2010 at 23:34

The sequences table is a hang over from D5 and will be in sites converted from D5. There is a sequences table in the site I use to test the conversion and some of the sequences are from add on modules but the last update to the table was a long time ago suggesting the add on modules changed their way of generating sequences after the D5-D6 conversion. I followed the Drupal conversion documentation for my modules, some of which used sequences, but there was never a point where the documentation said to delete the sequences table and there was never a D6 update to delete the table. We should at least mention the change in the conversion documentation.

My understanding is we have to use the sequence to handle databases without automatic sequencing. If that is the case how do add on modules fit in? In http://drupal.org/node/563106#comment-2524762 I suggested a way to find the maximum current sequence. I now realise it would not work for add on modules that changed to their own sequences. Suppose my Drupal sequence is up to 200 and the conversion converts an add on module that includes a table with the sequence up to 250. Does Drupal increase the sequence to 250? Does the module conversion have to use db_next_id to update the Drupal sequence? In the example the add on module could call db_next_id(250) if D7 does not do something automatically.

Last test case and one worth documenting. I used a central sequence for one site and it hit the 2 billion limit of a signed integer. While that figure seems high, it was a simple result of logging audit points for data progressing through workflow combined with a daily product status update of several hundred thousand products. I changed the site to unsigned integers then to separate sequences. I suggest, if everyone is using the sequences table, to document the limit in a page somewhere and link to the page from each database page that mentions sequences and ids.

The page could be called Sequences for large databases. It should describe the limitations on automatic sequences by database, sequence processing generated by specifying a sequence in the schema, the impact of using db_next_id, and a way to manually sequence.

While we are creating a patch for sequences, why not rename the table to sequence because it is the new table name format and I cannot find a table of that name in any D6 site. The name sequences could be reserved for future use when large sites request separate sequences per table.

Comment #12

anarcat CreditAttribution: anarcat commented 19 February 2010 at 23:56

Look: db_next_id() was *removed* from Drupal 6. So I don't know how modules could have used that in D6, but if they did, it was a bug and a crude local customization.

I think the implementation of sequences is outside the scope of this issue: there is now a *new* sequences table in D7 that is incompatible with the old one, and this issue is not about how that table is built or used, it's about getting rid of the old one so the new one works.

I really don't see what renaming actually gains us: people will have to fix their module to use the new name, why don't they port their module to the proper new API instead?

Comment #13

peterx CreditAttribution: peterx commented 20 February 2010 at 00:48

db_next_id is listed in d6 and is still listed in D7: http://api.drupal.org/api/function/db_next_id/7
The example for the function is After a database import, it might be that the sequences table is behind. That is a case that could occur in several of my sites where there are daily or regular imports from other systems. I will have to look through them to check the individual code.

An alternative is to replace db_next_id with Multi-insert, http://drupal.org/node/310079, and I suggested a documentation change to add a note linking from db_next_id to multi-insert so that people will consider multi-insert first. Most of my site imports, perhaps all, will convert to multi-insert.

There is nothing to say db_next_id will go away and the generic description of the function suggests you could use the id for things not related to the database. You could use it to identify external communications, message ids, payments, RPC calls, and similar, that would not be sequence fields in your own tables. If db_next_id is not going to officially go away, we need to handle it or document it for the conversion.

Comment #14

ctmattice1 CreditAttribution: ctmattice1 commented 20 February 2010 at 01:52

This is described in #563106: Cannot upgrade from Drupal 6 to Drupal 7 - meta issue

duplicate "system_list" index

Comment #15

ctmattice1 CreditAttribution: ctmattice1 commented 20 February 2010 at 02:04

I had a faulty patch at one time that renamed the tables. catch listed some of the modules that still used sequence if I remember correctly, maybe it was queue. renaming the tables was only in the patch so that it would help in the process of porting if your unfamiliar with the faulty modules functionality.

Could be dropped but wouldn't hurt to leave them in there for a while then drop them later, say around 7.5 or 8x

Comment #16

anarcat CreditAttribution: anarcat commented 22 February 2010 at 17:58

db_next_id is *not* in D6, I don't know where you get this idea: http://api.drupal.org/api/search/6/api/function/db_next_id

I know that db_next_id() is in D7, I'm not proposing it goes away or anything. My point here is that the sequences table was officially dropped in D6 and any module that relies on it in D6 is broken and should be shot or ported to D6 properly.

Renaming the table to _old will not help in any way, as the stupid D6 module will still look for the sequences table and will barf on the new D7 structure.

There is some work associated to porting modules between major drupal versions. D5 to D6 involved dropping the sequences table. So we *can* kill it now. D6 to D7 will allow contrib modules to use a *new* sequences table, so we *need* to kill the old table. There's no other way.

Comment #17

anarcat CreditAttribution: anarcat commented 22 February 2010 at 18:35

Assigned:

anarcat

» Unassigned

I'm done with this bug: i consider the table should be dropped, and I am open to further discussions about this, but I will not work on more patches.

Please do try the patch and the upgrade and set to RTBC if it works for you, both on d5/d6 sites and virgin d6 sites. I tried both and they both work fine.

Comment #18

ctmattice1 CreditAttribution: ctmattice1 commented 22 February 2010 at 19:29

File	Size
719730_3.patch	1.25 KB

Yes 719730.patch works on both D5->D6 and D6 updates

In Reviewing the patch

#719730 - fix queue and sequence tables upgrades from D5

diff --git includes/update.inc includes/update.inc
index 70861a6..c2d4f76 100644
--- includes/update.inc
+++ includes/update.inc
@@ -527,6 +527,13 @@ function update_fix_d7_requirements() {
         'expire' => array('expire'),
       ),
     );
+    // Check for queue table that may remain from a D5 insatll, if found rename
+    // it, it has a different schema than D7 but could be useful for upgrading
+    // modules.
+    if (db_table_exists('queue')) {
+      db_drop_table('queue');
+    }
+
     db_create_table('queue', $schema['queue']);

Comment should be changed to

// Check for queue table that may remain from D5 or D6, if found
//drop it.

     // Create the sequences table.
@@ -542,6 +549,12 @@ function update_fix_d7_requirements() {
        ),
       'primary key' => array('value'),
     );
+    // Check for sequences table that may remain from a Drupal 5 install, if
+    // found rename it, it has a different schema than D7 but could be useful
+    // for upgrading modules.
+    if (db_table_exists('sequences')) {
+      db_drop_table('sequences');
+    }
     db_create_table('sequences', $schema['sequences']);
     // Initialize the table with the maximum current increment of the tables
     // that will rely on it for their ids.

same here change the comments to

// Check for sequences table that may remain from D5 or D6, if found
//drop it.

Patch attached

Comment #19

22 February 2010 at 19:40

Status:

Needs review

» Needs work

The last submitted patch, 719730_3.patch, failed testing.

Comment #20

peterx CreditAttribution: peterx commented 23 February 2010 at 09:44

File	Size
719730_4.patch	1.21 KB

719730_3.patch with /r/n changes to /n.

Comment #21

peterx CreditAttribution: peterx commented 23 February 2010 at 10:06

I tried this patch with alpha2 on a D5->D6 site and update.php ran through to the update log where 7003 and 7004 had errors. Looks good. No WSOD.

Comment #22

andypost

he/him

Russian

CreditAttribution: andypost commented 23 February 2010 at 16:09

Status:

Needs work

» Needs review

It seems we should check for properly updated D5 => D6 before starting D6 => D7

D5 => D6 is not ideal so should we fix D6 updates or bring this checks to D7...

Comment #23

anarcat CreditAttribution: anarcat commented 24 February 2010 at 02:49

Status:

Needs review

» Reviewed & tested by the community

So we're agreeing here? We drop the table? Since the patch passes testing, has been tested by two other people, I'm marking this RTBC.

Comment #24

dries CreditAttribution: dries commented 24 February 2010 at 10:02

I think this looks good so I'll commit this patch later today.

- D7 has a queue table but it is different from the one in D5, which came with the queue module (moderation queue). It looks like we never dropped the D5 table. It is safe to drop this table; the Drupal 6 version of the queue module in contrib needs to take over this table, and rename it in preparation for D7. We should open an issue against the queue module to inform the maintainer of this requirement.

- D7 has a sequences table but it is different from the one in D5. From what I can tell, dropping the old table prior to creating the new table is correct.

The patch has two small code style issues -- missing space after the second //s. I can fix this prior to the commit, but feel free to reroll the patch if you want.

Comment #25

dries CreditAttribution: dries commented 25 February 2010 at 09:43

Status:

Reviewed & tested by the community

» Fixed

I've now committed this patch to CVS HEAD. Thanks all. Great work.

Comment #26

11 March 2010 at 09:50

Status:	Fixed	» Closed (fixed)
Issue tags:	-D7 upgrade path, -D7csmtl

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

Comment #27

11 March 2010 at 09:50

Issue tags:

+D7 upgrade path, +D7csmtl

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.

drop the sequences and queue tables from D5 during the D6 -> D7 upgrade

Comments

Comment #1

Comment #2

Comment #3

Comment #4

Comment #5

Comment #6

Comment #7

Comment #8

Comment #9

Comment #10

Comment #11

Comment #12

Comment #13

Comment #14

Comment #15

Comment #16

Comment #17

Comment #18

Comment #19

Comment #20

Comment #21

Comment #22

Comment #23

Comment #24

Comment #25

Comment #26

Comment #27

News items

Our community

Documentation

Drupal code base

Governance of community