WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 7 months ago

#15004 closed defect (bug)

Missing index on signups table — at Version 9

Reported by: barry Owned by:
Milestone: 3.7 Priority: normal
Severity: normal Version:
Component: Database Keywords: has-patch commit needs-refresh 3.6-early
Focuses: Cc:

Description (last modified by nacin)

wp-includes/ms-functions.php:590:
$signup = $wpdb->get_row( $wpdb->prepare("SELECT * FROM $wpdb->signups WHERE user_email = %s", $user_email) );
wp-includes/ms-functions.php:595:
$wpdb->query( $wpdb->prepare("DELETE FROM $wpdb->signups WHERE user_email = %s", $user_email) );

But there is no index on user_email in the signups table. Makes these queries perform a full table scan which is slow when you have lots of signups.

Attached patch adds the index but I can't figure out how schema upgrades on MS-specific tables ever get run after the initial activation of MS mode and table creation.

Change History (11)

barry4 years ago

comment:1 barry4 years ago

  • Summary changed from Missing index on wp_signups to Missing index on signups table

comment:2 nacin3 years ago

  • Keywords 3.2-early commit added
  • Milestone changed from Awaiting Review to Future Release

comment:3 SergeyBiryukov3 years ago

  • Keywords 3.2-early removed
  • Milestone changed from Future Release to 3.3

comment:4 in reply to: ↑ description nacin3 years ago

Replying to barry:

Attached patch adds the index but I can't figure out how schema upgrades on MS-specific tables ever get run after the initial activation of MS mode and table creation.

They don't. We'll need to call install_network() in upgrade.php. Possibly only in an upgrade_XYZ() function rather than on every upgrade.

Ryan, would it be safe to move install_network() to schema.php and populate_network() to upgrade.php? It bothers me that those functions are in the opposite files.

comment:5 ryan3 years ago

Seems safe. It bothers me too that they are flipped.

comment:6 ryan3 years ago

  • Milestone changed from 3.3 to Future Release

comment:7 nacin22 months ago

  • Milestone changed from Future Release to 3.5
// Upgrade global tables only for the main site. Don't upgrade at all if DO_NOT_UPGRADE_GLOBAL_TABLES is defined.
if ( in_array( $table, $global_tables ) && ( !is_main_site() || defined( 'DO_NOT_UPGRADE_GLOBAL_TABLES' ) ) )
	continue;

So, dbDelta() is already aware that we should avoid upgrading global tables except when operating on the main site (and the danger flag isn't set). It also looks like make_db_current_silent() will trigger an upgrade for wp_get_db_schema( 'all' ), which includes global tables. Haven't tested, but it appears schema changes to global tables will now get picked up just fine. All we'd need is a refresh of 15004.diff and a DB version bump.

comment:8 nacin22 months ago

  • Keywords has-patch added

Before:

id select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE wp31beta_signups ALL 30 Using where

After:

id select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE wp31beta_signups ref user_email user_email 302 const 1 Using where

(Separately, you know what's fun? No primary key on the signups table.)

New patch properly names the key and refreshes it against trunk.

nacin22 months ago

comment:9 nacin22 months ago

  • Description modified (diff)
Note: See TracTickets for help on using tickets.