-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Unique indexes #1971
Unique indexes #1971
Conversation
Current coverage is 91.98%
@@ master #1971 diff @@
==========================================
Files 91 91
Lines 6420 6449 +29
Methods 1096 1108 +12
Messages 0 0
Branches 1347 1358 +11
==========================================
+ Hits 5901 5932 +31
+ Misses 519 517 -2
Partials 0 0
|
@DrewGross if we go for 2.3.0 let's put the upgrade guide in that PR, maybe in a 2.3.0.md and move it to the README.md when releasing? |
Good idea. I was going to put it in the Parse Blog, but having it in the repo sounds like a good idea too. |
@drew-gross updated the pull request. |
1 similar comment
@drew-gross updated the pull request. |
At least 2 changes are coming: Don't attempt to create the index every time a user signs up, and don't delete null emails if the index build fails. |
3107e97
to
8788a0e
Compare
@drew-gross updated the pull request. |
@drew-gross updated the pull request. |
16 similar comments
@drew-gross updated the pull request. |
@drew-gross updated the pull request. |
@drew-gross updated the pull request. |
@drew-gross updated the pull request. |
@drew-gross updated the pull request. |
@drew-gross updated the pull request. |
@drew-gross updated the pull request. |
@drew-gross updated the pull request. |
@drew-gross updated the pull request. |
@drew-gross updated the pull request. |
@drew-gross updated the pull request. |
@drew-gross updated the pull request. |
@drew-gross updated the pull request. |
@drew-gross updated the pull request. |
@drew-gross updated the pull request. |
@drew-gross updated the pull request. |
217ec56
to
1a0859d
Compare
@drew-gross updated the pull request. |
@drew-gross updated the pull request. |
@drew-gross updated the pull request. |
1 similar comment
@drew-gross updated the pull request. |
@drew-gross updated the pull request. |
I just had a look at the new readme file + the new code directly dealing with creating those indexes and it looks good to me. I'd look into why the tests are failing (seems just flaky?) and maybe have someone how knows what they're doing with JS check the rest of the changes. For the unique index creation part, LGTM. |
One of the other changes in here causes tests to fail if there are any open connections to the server left after the test finishes. This was really helpful for debugging, but a lot of our other tests leave connections open so until we can clean them up, they will be flaky. Fortunately it's easy to detect and fix the flakiness now. |
Huge PR.
Adds a way to create unique indexes, and use this new method for ensuring that usernames and emails are unique. But, some people might already have non-unique emails and username, so if the index build fails, we fall back on the previous method. I think I'll make this trigger 2.3.0 and encourage people to build their unique indexes before upgrading.
For behaviour around nulls: Unique indexes are required to ignore undefined/unset fields for the purposes of uniqueness. This is why we use sparse indexes. For null, the plan is to not allow unique indexes on nullable fields. Since we don't currently have a way to specify that a field is non-nullable, we basically aren't going to expose this interface to users yet.
We already require unique emails, but both null and undefined don't count towards uniqueness, somehow, on Parse.com. I think if anyone actually has null for multiple emails, we'll tell them to make sure to clean up their data before upgrading to 2.3.0.