[POOL-608] allow for case insensitive users search by email #1283
Conversation
|
I believe we will need a partybus migration to deploy the new sqitch index on upgrades. |
srenatus
force-pushed
the
sr/pool-608/search-users-by-case-insenstive-email
branch
from
b358900
to
50c6112
Compare
srenatus
changed the title
|
We've come to the conclusion that this is a cumbersome approach, and that it might be easier to get a migration in place for changing email's column type from I'll put this branch on hold and start another one for the migration. |
srenatus
force-pushed
the
sr/pool-608/search-users-by-case-insenstive-email
branch
from
50c6112
to
8d50b22
Compare
|
After some more discussion, it might be the case that the best compromise is as follows:
That way we don't need a migration. Also, we wave off any guarantees of duplicate entries not happening -- they may be there already (and we can't fix those), so they may be added as well. They just may be there. |
|
This would be good to go. Just waiting for chef/chef-zero#265 to merge so we can drop 8d50b22. |
srenatus
force-pushed
the
sr/pool-608/search-users-by-case-insenstive-email
branch
from
8d50b22
to
f401d6e
Compare
|
The chef-zero change this depends on has been merged ✔️ The only missing bit is asking ops if creating that index will be fine as-is for hosted. ✔️ |
|
|
||
| -- TODO 2017/05/30 sr: check with ops if this should include CONCURRENTLY | ||
| -- https://www.postgresql.org/docs/9.2/static/sql-createindex.html#SQL-CREATEINDEX-CONCURRENTLY | ||
| CREATE INDEX users_lower_email_idx ON users (lower(email)); |
There was a problem hiding this comment.
Perhaps we should only create the index if it doesn't exist, that way ops can deploy it different if they choose to and this will still be safe to run.
There was a problem hiding this comment.
Uhm, yeah, that's a good idea. CREATE INDEX IF NOT EXISTS it shall be.
There was a problem hiding this comment.
Unfortunately, I think CREATE INDEX IF NOT EXISTS isn't supported in 9.2. This stack-overflow answer presents alternatives for different versions:
There was a problem hiding this comment.
There was a problem hiding this comment.
It would have been sooo nice. But thanks! 😃
srenatus
force-pushed
the
sr/pool-608/search-users-by-case-insenstive-email
branch
from
e227807
to
4069b32
Compare
srenatus
changed the title
|
I don't believe this has to be held back anymore. The index creation is done concurrently now, and the corresponding verification disabled. No user should be hurt in the process. |
srenatus
force-pushed
the
sr/pool-608/search-users-by-case-insenstive-email
branch
from
4069b32
to
60f00a2
Compare
|
cc @chef/chef-server-maintainers |
marcparadise
force-pushed
the
sr/pool-608/search-users-by-case-insenstive-email
branch
2 times, most recently
from
37b429e
to
bbef679
Compare
Also adds tests to pedant that failed before. Signed-off-by: Stephan Renatus <srenatus@chef.io>
Signed-off-by: Stephan Renatus <srenatus@chef.io>
Signed-off-by: Stephan Renatus <srenatus@chef.io>
Signed-off-by: Stephan Renatus <srenatus@chef.io>
This doesn't work with PG 9.6. Signed-off-by: Stephan Renatus <srenatus@chef.io>
marcparadise
deleted the
sr/pool-608/search-users-by-case-insenstive-email
branch
So far, this is for
GET /users?email=foo%40bar.com, and adding an index to make this not super slow (not verified if it's strictly necessary, so if tests in hosted reveal that we don't need this, we could drop b358900).It doesn't yet forbid creating a second user with the same email (case sensitively).