Ticket #4027 (new defect)

Opened 1 year ago

Last modified 1 week ago

Upgrade function don't do ANY error checking whatsoever

Reported by: skjaeve Assigned to:
Priority: high Milestone: 2.6
Component: Administration Version: 2.1.2
Severity: critical Keywords: early needs-patch
Cc: scott@scott-h.com

Description

When I upgraded from 2.0.5 to 2.1.2, there was an error in my MySQL privileges, so the ALTER TABLE commands failed: 

    WordPress database error: [ALTER command denied to user 'aasmunds'@'localhost' for table 'wp_categories']
    ALTER TABLE wp_categories ADD COLUMN link_count bigint(20) NOT NULL default '0'

    WordPress database error: [ALTER command denied to user 'aasmunds'@'localhost' for table 'wp_categories']
    ALTER TABLE wp_categories ADD COLUMN posts_private tinyint(1) NOT NULL default '0'

    WordPress database error: [ALTER command denied to user 'aasmunds'@'localhost' for table 'wp_categories']
    ALTER TABLE wp_categories ADD COLUMN links_private tinyint(1) NOT NULL default '0'

There were also a lot of secondary errors due to the above failed commands. However, the upgrade.php script still claimed that all was well: 

There's actually only one step. So if you see this, you're done. Have fun!

It would be more helpful if WP would detect the MySQL errors and actually say that something went wrong and must be fixed, instead of saying that all is well even though there were heaps of errors.

This only happens once: If I run the upgrade.php script again, it just says 'all is well' without any errors, so the MySQL commands that would fail are not run again. However, the database is still broken.

It would be better if WP discovered that an upgrade failed, and set a flag somewhere that said 'this upgrade must be run again'.

See also http://wordpress.org/support/topic/111398?replies=2

Change History

03/25/07 17:05:11 changed by rob1n

  • owner changed from anonymous to rob1n.
  • status changed from new to assigned.

Looking.

03/25/07 17:14:40 changed by rob1n

  • owner deleted.
  • priority changed from low to high.
  • status changed from assigned to new.
  • severity changed from normal to major.
  • milestone changed from 2.3 to 2.2.

There is currently no checking that wp_upgrade() or its associated functions failed. I'm not sure what to do about this, as having an actual error reporting system would require rewriting parts of some of the functions.

Perhaps we could make use of WP_Error?

03/25/07 17:20:15 changed by rob1n

  • severity changed from major to critical.
  • summary changed from upgrade.php doesn't detect that ALTER TABLE fails to Upgrade function don't do ANY error checking whatsoever.

04/12/07 06:13:43 changed by rob1n

  • milestone changed from 2.2 to 2.3.

04/12/07 15:43:12 changed by markjaquith

We definitely should not be incrementing the DB version number if the upgrade commands fail, as that prevents the upgrade from working once any MySQL permissions issues are ironed out. Would also be nice to present the errors in a friendly way that points to solutions, but I think the priority here is making sure that once resolved, the upgrade will re-run and bring everything up-to-date.

07/21/07 16:36:36 changed by Nazgul

  • milestone changed from 2.3 (trunk) to 2.4 (future).

03/07/08 14:41:49 changed by westi

  • keywords set to early needs-patch.
  • milestone changed from 2.5 to 2.6.

I think it's a bit late to go playing with the upgrade code for 2.5 punting to 2.6

06/25/08 18:21:27 changed by Scott H

  • cc set to Scott, H.

Any discussion on how this could be implemented? Should failed queries cause the script to bail? This would fix a few of the problems by displaying an error but probably doesn't help the upgrade process and may interfere elsewhere? (I've been trying to think of where a query error shouldn't lead to a fatal error).

06/25/08 18:47:28 changed by Scott H

  • cc changed from Scott, H to scott@scott-h.com.