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

Make username case insensitive #243

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Forceu
Copy link

@Forceu Forceu commented Jan 9, 2021

When logging in by username, the username was case-sensitive. This PR makes it case-insensitive

@ocram
Copy link
Contributor

ocram commented Jan 30, 2021

Thanks!

That’s indeed the behavior we want (case-insensitive).

What database system are you using though? SQLite, I guess?

As far as I remember, that COLLATE NOCASE in a query would be specific to SQLite, not part of the standard, and incompatible with the other database systems that we support. So we can’t add it there.

But can’t we add COLLATE NOCASE to the column definitions (of the few affected columns) instead? That would mean we change the schema in Database/SQLite.sql instead. This could also be a little faster (because indexes are built the right way).

What do you think?

@Forceu
Copy link
Author

Forceu commented Jan 31, 2021

Yes, I am using SQLite3 indeed. Sorry, I thought this statement was only for sqlite. In that case it is probably best to use your way and I will close this PR. Thanks for the feedback! :)

@Forceu Forceu closed this Jan 31, 2021
@ocram
Copy link
Contributor

ocram commented Jan 31, 2021

No problem!

Could you test the alternative solution of using COLLATE on the columns in the schema (and removing it from the query again)? Or aren’t you interested in this anymore (at the moment)?

(I’ll leave this open until any solution for this problem has been implemented.)

@ocram ocram reopened this Jan 31, 2021
@Forceu
Copy link
Author

Forceu commented Jan 31, 2021

I will test it later this week!

@ocram
Copy link
Contributor

ocram commented Feb 1, 2021

Thanks!

It’s the three columns

users.email
users.username
users_confirmations.email

where the COLLATE clause would have to be added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants