Create index on user_email

[chriscct7]

In order to improve performance, an index could be added on user_email (indexes already exist on user_login and user_nicename), to make it faster to query against.

In addition, the db structure could be altered to add a UNIQUE constraint on user_nicename, user_email, and user_nicename for those columns on the wp_users table, which would improve performance when a site has many users, and eliminates to run a pre-insert SELECT check when inserting users.

The patch attached adds the schema changes proposed which would apply to new installs automatically, as well as the upgrade routine for existing installs (though the db_version compared against would likely need to be altered by the time this could get merged).

In addition to the performance improvements, this would also eliminate the chance of race conditions such as proposed by #30959.

[knutsp]

This may fail for user_email, as a plugin could have allowed users with the same email.

[chriscct7]

Replying to knutsp:

This may fail for user_email, as a plugin could have allowed users with the same email.

Replying to knutsp:

This may fail for user_email, as a plugin could have allowed users with the same email.

To do so would either require direct SQL calls or overriding tons of checks for email_exists in order to avoid the fact that WordPress prevents users from sharing an email address. I'm sure a plugin could be devised, if so desired, that could break on any nearly any change to WordPress core.

A plugin that allows the same email to be shared would also break other issues like #9568 (login via an email address; which user gets logged in when they share an email?), as well as developers ability to count on predicable results to core functions like get_user_by('email', $value). As such, changing the tables to not support duplicates simply complements checks WordPress already has.

If it was desired, we could skip the upgrades on existing installs (patch for that attached), but in any case. A better option would be to query all rows where duplicates exist and update the emails to be username+{email}. Patch also attached for this.

[chriscct7]

33376 without updating existing sites

[chriscct7]

33376 with deduplication of user_email routine

[DrewAPicture]

Replying to chriscct7:

Replying to knutsp:

This may fail for user_email, as a plugin could have allowed users with the same email.

To do so would either require direct SQL calls or overriding tons of checks for email_exists in order to avoid the fact that WordPress prevents users from sharing an email address. I'm sure a plugin could be devised, if so desired, that could break on any nearly any change to WordPress core.

A plugin that allows the same email to be shared would also break other issues like #9568 (login via an email address; which user gets logged in when they share an email?), as well as developers ability to count on predicable results to core functions like get_user_by('email', $value). As such, changing the tables to not support duplicates simply complements checks WordPress already has.

The ​Allow Multiple Accounts plugin by @coffee2code has been a popular choice for years for allowing multiple accounts with the same email. So it's not really unheard of.

And for what it's worth, it leverages the pre_user_email hook to do it.

[chriscct7]

While I agree it's not unheard of (Scott's plugin is a great example), the question is whether the few users doing something WordPress doesn't natively support (5k out of many many millions) and in fact has code to prevent against are enough reason to block a potential performance impact for all of the other users (the clear majority of WordPress user's don't do that), which would be very significant on large sites with many users (particularly when going a get user by email call). While I admit if this goes in it makes a plugin like Scott's a bit harder to accomplish, it wouldn't be impossible (you could for example store users in a custom user table or add a table that maps additional usernames to an email, or something along those lines. And the third provided patch takes care of users who already share an email via a plugin like that.

[knutsp]

If this goes in, please use localpart+username@… to make duplicate email unique and not vice versa. Gmail supports localpart+somestring@… as aliases for localpart@…. Then, for all users with Gmail, and maybe more, this will work out of the box.

Note:
My main business' main system depends on "Allow Multiple Accounts" plugin (case: same employee sometimes employed by several employers, but system currently only allows each employee to attach to one employer). We will adapt and change our system, but I think a warning should be given months ahead. If I spot trouble here, I guess many will have.

Core has allowed duplicate emails on users for a decade (?), even if it's not the internal default and intention.

I do understand and respect the reasons for this (proposed) change, though.

[chriscct7]

Replying to knutsp:

If this goes in, please use localpart+username@… to make duplicate email unique and not vice versa. Gmail supports localpart+somestring@… as aliases for localpart@…. Then, for all users with Gmail, and maybe more, this will work out of the box.

The patch does exactly this :-) so no worries there.

Core has allowed duplicate emails on users for a decade (?), even if it's not the internal default and intention.

Core actually hasn't supported this. Calling any register user function checks fora unique email and has for a while. Plugins like multiple email employ methods that work around all of WordPress's internal checks.

That being said this is still a proposal and there is alot more that will be done on this. If you subscribe to the ticket, you'll get an email anytime someone comments or the status of this proposal changes.

[jorbin]

In theory, these changes sound like they will improve performance, but I would like to see some actual benchmarks vs both MyISAM and Innodb.

The unique changes for one worry me. While core hasn't directly supported non-unique user_email, it is something that I know many installs have supported. I would be especially hesitant to essentially remove the ability for this to function without some clear numbers showing a lot of benefit.

[dd32]

Unfortunately I feel the UNIQUE index is probably out of reach for us, and since we're already enforcing uniqueness at the application layer, a unique index vs non-unique index doesn't seem like the performance benefit would outweigh any compatibility issues.

[chriscct7]

Yeah at this point UNIQUE is out of range. Index could still be done

[ocean90]

#35909 was marked as a duplicate.

[ocean90]

Replying to DrewAPicture:

The ​Allow Multiple Accounts plugin by @coffee2code has been a popular choice for years for allowing multiple accounts with the same email. So it's not really unheard of.

That's an interesting plugin. I'm curious if any of the sites are using a persistent object cache because there is a useremail cache group which gets used by WP_User::get_data_by().

cc: @pento

[pento]

WP.com has a UNIQUE index on user_email, but I'm inclined to agree that we can't practically do that for all sites. Also, enforcing UNIQUE keys has burned us in the past when we later decide to make something non-UNIQUE.

The performance difference between UNIQUE and non-UNIQUE indexes is negligible, so let's commit 33376.2.patch​ (with $wp_db_version bump, of course).

[ocean90]

In 36654:

Schema: Add an index to wp_users.user_email.

Improves lookup of an email address on large user tables.

See #9568.
Fixes #33376.