Missing index on signups table

[barry]

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.

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.

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

// 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.

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.

user_login also has a query running against it, also in wpmu_validate_user_signup(). Both fields should be indexed.

(user_login,user_email) is a reasonable PK. I also noticed a query that was deleting by (domain,path), so swapping the domain key for domain_path will improve that.

As adding a PK will fail if we have duplicate rows (and, well, who knows), let's go with a user_login,user_email key. Looks good otherwise.

Agreed, fixed in patch 5.

I think we need to remove the domain key via a separate ALTER in the upgrade routine.

Removal of the domain key would happen in pre_schema_upgrade(), if is_multisite() && is_main_site() && ! defined( 'DO_NOT_UPGRADE_GLOBAL_TABLES' ).

We need a way to not fire the DROP INDEX for this for every main site of a multi-network. There's no harm in running a DROP INDEX on an index that doesn't exist (we will need to suppress errors) or we can first check if the index is there before dropping it.

We can do this prior to beta 2.

Replying to nacin:

We can do this prior to beta 2.

If ryan doesn't mind. It's a bit unorthodox for us to do schema changes anytime but super early in the cycle.

pento, in IRC: "I'd be inclined to do a SHOW INDEX. Suppressing errors would be bad if there was an error we cared about."

We long passed the appropriate time for a schema change.

Closed as a duplicate — #24279, for adding a primary key here.