Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert case sensitivity for email uniqueness #930

Merged
merged 1 commit into from
Feb 26, 2021

Conversation

sidonath
Copy link
Contributor

@sidonath sidonath commented Feb 3, 2021

Hi everyone and thanks for maintaining the gem ❤️

Unfortunately, we ran into an issue after we updated Clearance to the latest version: some of our requests started failing due to statement timeouts in the database.

After a closer inspection it turned out the problem was related to queries on the users table which were changed to query by LOWER(email) instead of email. Since we didn't have the index on LOWER(email), the queries for some users got significantly slower.

This is related to the change made in #889case_sensitive setting of the uniqueness validator was set
to false to prevent deprecation warnings from Rails.

However, the setting should have been set to true to preserve backwards-compatibility.

The deprecation warning said:

To continue case sensitive comparison
on the :email attribute in User model, pass case_sensitive: true
option explicitly to the uniqueness validator.

The problem was mentioned in an old commit that originally made the switch from the explicit false to the implicit true:
56f0f04

If I might recommend pushing this change in a bugfix release and if case_sensitive: false is indeed desired, push that change in Clearance 3.0.

In thoughtbot#889, `case_sensitive` setting of the `uniqueness` validator was set
to `false` to prevent deprecation warnings from Rails.

However, the setting should have been set to `true` to preserve
backwards-compatibility.

The deprecation warning said:

> To **continue** case sensitive comparison
> on the :email attribute in User model, pass case_sensitive: true
> option explicitly to the uniqueness validator.

Setting it to `false`, made ActiveRecord generate User queries on
`LOWER(email)` instead of on `email`. The apps that didn't have an index
on `LOWER(email)` would have faced performance issues.

This was mentioned in an old commit that originally made the switch from
`false` to the implicit `true`:
thoughtbot@56f0f04
@rtymchyk
Copy link
Contributor

rtymchyk commented Feb 8, 2021

This would explain why our end-to-end tests suddenly started seeing network timeouts with the latest version.

@MottiniMauro
Copy link
Contributor

Hi @sidonath , thanks for the contribution! We are merging to release it in the next bugfix release.

@MottiniMauro MottiniMauro merged commit b50defc into thoughtbot:master Feb 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants