slow query in email_registration_user_login_validate

More specifically the query is:

    if ($name = db_result(db_query("SELECT name FROM {users} WHERE LOWER(mail) = LOWER('%s')", $form_state['values']['name']))) {

So, if we just get rid of the LOWER() functions this will 1) still work on mysql 2) get to use the index.

Here is the explain statement:

mysql> explain SELECT name FROM users where mail = 'greg@example.com';
+----+-------------+-------+------+---------------+------+---------+-------+------+-------------+
| id | select_type | table | type | possible_keys | key  | key_len | ref   | rows | Extra       |
+----+-------------+-------+------+---------------+------+---------+-------+------+-------------+
|  1 | SIMPLE      | users | ref  | mail          | mail | 195     | const |    1 | Using where | 
+----+-------------+-------+------+---------------+------+---------+-------+------+-------------+
1 row in set (0.13 sec)

mysql> explain SELECT name FROM users where lower(mail) = lower('greg@example.com');
+----+-------------+-------+------+---------------+------+---------+------+---------+-------------+
| id | select_type | table | type | possible_keys | key  | key_len | ref  | rows    | Extra       |
+----+-------------+-------+------+---------------+------+---------+------+---------+-------------+
|  1 | SIMPLE      | users | ALL  | NULL          | NULL | NULL    | NULL | 3527497 | Using where | 
+----+-------------+-------+------+---------------+------+---------+------+---------+-------------+

Note how the first query without the lower gets a possible_key of mail and only has to scan 1 row. The second query has to scan all 3 million+ records :(

The attached patch creates a variable that has no user interface, but it can be used based on the documentation in the new README.txt

Here's a patch based on #3.

Also, I'm going roughly with option B from #3. One reason I don't like the database type test is that it's not really accurate to say that "mysql is case insensitive." Mysql's defaults are to be case insensitive but it's possible to make a column case sensitive (as was done to drupal.org usernames for a while). Making it a variable also lets someone optimize the query for their own mongo, sqlite, oracle, sql server, etc.

Well, that was using the wrong query by default. We should default to the version without lower (right?).

That's fine, I'm not personally affected by this and don't plan to commit it in the near future unless a clear consensus comes.

While you're right that getting the query from a variable is not common it's also not common to use the global db type to flip around.

I'm not really familiar with db_rewrite_sql being used for performance purposes - do you have any examples of that?

OK, let's revive this. It's now affecting me :)

In #279851: Replace LOWER() with db_select() and LIKE() where possible there was a system created for cross-db compatible case insensitive queries. It's documented at https://www.drupal.org/update/modules/6/7#nomorelower which suggests this change:

Drupal 6:

 $result = db_query_range("SELECT value FROM {profile_values} WHERE fid = %d AND LOWER(value) LIKE LOWER('%s%%')", $field, $string, 0, 10);

Drupal 7:

$result = db_select('profile_field')
    ->fields('profile_field', array('category'))
    ->condition('category', db_like($string) . '%', 'LIKE')
    ->range(0, 10)
    ->execute();

So...let's do that for d7.

Also, there's no sense in running the query if the content of the field is not a valid email address, so I've added a check to see if the field is an email. I also moved around some of the logic of the function to keep some of the other features working (I think I got it right, would love a review because I don't use all the features of the module).

I've now tested this locally and it seems to work with email_registration_login_with_username both true and false.

I also confirmed that the query is greatly improved.

Here's a slightly updated version of the patch with some whitespace removed and a comment clarified.

I had just copy/pasted the db_like without thinking about it too much, but @yesnoio asked whether that was right, so I tested.

I created a user 1 with the email admin@example.com and uid 3 with adm_n@example.com. Without the db_like, the module can get confused about whether the user that is trying to login is 1 or 3. I don't think this is really a security issue, because the password still has to be guessed, but it's not optimal. With the db_like, the condition for adm_n@example.com becomes "mail LIKE 'adm\_n@example.com'". For funzies, here's some queries showing the different behavior:

mysql> select uid, mail from users where mail LIKE 'adm_n@example.com';
+-----+-------------------+
| uid | mail              |
+-----+-------------------+
|   1 | admin@example.com |
|   4 | adm_n@example.com |
+-----+-------------------+
2 rows in set (0.00 sec)

mysql> select uid, mail from users where mail LIKE 'adm\_n@example.com';
+-----+-------------------+
| uid | mail              |
+-----+-------------------+
|   4 | adm_n@example.com |
+-----+-------------------+
1 row in set (0.01 sec)

mysql>

OK, committed that.

Drupal 8 uses `user_load_by_mail` and honestly I'm not sure why Drupal 7 doesn't do that....Anyway, seems this status should be fixed since there's nothing to port forward.