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

fix: improve perf in account linking #1394

Merged
merged 2 commits into from
Feb 5, 2024
Merged

Conversation

kangmingtay
Copy link
Member

@kangmingtay kangmingtay commented Feb 2, 2024

What kind of change does this PR introduce?

  • Currently, we use an ilike which doesn't use the index on auth.identities.email. Since the emails in the verifiedEmails slice only contains lowercased emails, and the auth.identities.email are also guaranteed to be lowercase, there is no need for a case-insensitive search
  • The partial unique index (users_email_partial_key) in auth.users is defined on the condition that is_sso_user = false. However, the query uses is_sso_user is false which causes it to not use the index ever (along with ilike). It would be much faster to do lower(email) = any (?) rather than email ilike any (?)
  • Fixes Account linking queries do a sequential scan on the users/identities table by default in managed Supabase #1390

internal/models/linking.go Outdated Show resolved Hide resolved
J0
J0 previously approved these changes Feb 2, 2024
@J0 J0 dismissed their stale review February 2, 2024 09:31

Sounds like we might need to use a partial index, remove lower, or something else

@kangmingtay
Copy link
Member Author

decided to remove the case-insensitive search here since gotrue will lowercase all emails before inserting / updating a user record in the database

if we want to be really safe, we can add an index to enforce uniqueness on lower(email) but that will be for a FLUP PR

@kangmingtay kangmingtay merged commit 8eedb95 into master Feb 5, 2024
2 checks passed
@kangmingtay kangmingtay deleted the km/fix-improve-perf branch February 5, 2024 10:59
J0 pushed a commit that referenced this pull request Feb 12, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.139.2](v2.139.1...v2.139.2)
(2024-02-08)


### Bug Fixes

* improve perf in account linking
([#1394](#1394))
([8eedb95](8eedb95))
* OIDC provider validation log message
([#1380](#1380))
([27e6b1f](27e6b1f))
* only create or update the email / phone identity after it's been
verified ([#1403](#1403))
([2d20729](2d20729))
* only create or update the email / phone identity after it's been
verified (again)
([#1409](#1409))
([bc6a5b8](bc6a5b8))
* unmarshal is_private_email correctly
([#1402](#1402))
([47df151](47df151))
* use `pattern` for semver docker image tags
([#1411](#1411))
([14a3aeb](14a3aeb))


### Reverts

* "fix: only create or update the email / phone identity after i…
([#1407](#1407))
([ff86849](ff86849))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
uxodb pushed a commit to uxodb/auth that referenced this pull request Nov 13, 2024
## What kind of change does this PR introduce?
* Currently, we use an `ilike` which doesn't use the index on
`auth.identities.email`. Since the emails in the `verifiedEmails` slice
only contains lowercased emails, and the `auth.identities.email` are
also guaranteed to be lowercase, there is no need for a case-insensitive
search
* The partial unique index (`users_email_partial_key`) in auth.users is
defined on the condition that `is_sso_user = false`. However, the query
uses `is_sso_user is false` which causes it to not use the index ever
(along with `ilike`). It would be much faster to do `lower(email) = any
(?)` rather than `email ilike any (?)`
* Fixes supabase#1390
uxodb pushed a commit to uxodb/auth that referenced this pull request Nov 13, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.139.2](supabase/auth@v2.139.1...v2.139.2)
(2024-02-08)


### Bug Fixes

* improve perf in account linking
([supabase#1394](supabase#1394))
([8eedb95](supabase@8eedb95))
* OIDC provider validation log message
([supabase#1380](supabase#1380))
([27e6b1f](supabase@27e6b1f))
* only create or update the email / phone identity after it's been
verified ([supabase#1403](supabase#1403))
([2d20729](supabase@2d20729))
* only create or update the email / phone identity after it's been
verified (again)
([supabase#1409](supabase#1409))
([bc6a5b8](supabase@bc6a5b8))
* unmarshal is_private_email correctly
([supabase#1402](supabase#1402))
([47df151](supabase@47df151))
* use `pattern` for semver docker image tags
([supabase#1411](supabase#1411))
([14a3aeb](supabase@14a3aeb))


### Reverts

* "fix: only create or update the email / phone identity after i…
([supabase#1407](supabase#1407))
([ff86849](supabase@ff86849))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 13, 2024
## What kind of change does this PR introduce?
* Currently, we use an `ilike` which doesn't use the index on
`auth.identities.email`. Since the emails in the `verifiedEmails` slice
only contains lowercased emails, and the `auth.identities.email` are
also guaranteed to be lowercase, there is no need for a case-insensitive
search
* The partial unique index (`users_email_partial_key`) in auth.users is
defined on the condition that `is_sso_user = false`. However, the query
uses `is_sso_user is false` which causes it to not use the index ever
(along with `ilike`). It would be much faster to do `lower(email) = any
(?)` rather than `email ilike any (?)`
* Fixes supabase#1390
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 13, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.139.2](supabase/auth@v2.139.1...v2.139.2)
(2024-02-08)


### Bug Fixes

* improve perf in account linking
([supabase#1394](supabase#1394))
([8eedb95](supabase@8eedb95))
* OIDC provider validation log message
([supabase#1380](supabase#1380))
([27e6b1f](supabase@27e6b1f))
* only create or update the email / phone identity after it's been
verified ([supabase#1403](supabase#1403))
([2d20729](supabase@2d20729))
* only create or update the email / phone identity after it's been
verified (again)
([supabase#1409](supabase#1409))
([bc6a5b8](supabase@bc6a5b8))
* unmarshal is_private_email correctly
([supabase#1402](supabase#1402))
([47df151](supabase@47df151))
* use `pattern` for semver docker image tags
([supabase#1411](supabase#1411))
([14a3aeb](supabase@14a3aeb))


### Reverts

* "fix: only create or update the email / phone identity after i…
([supabase#1407](supabase#1407))
([ff86849](supabase@ff86849))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 15, 2024
## What kind of change does this PR introduce?
* Currently, we use an `ilike` which doesn't use the index on
`auth.identities.email`. Since the emails in the `verifiedEmails` slice
only contains lowercased emails, and the `auth.identities.email` are
also guaranteed to be lowercase, there is no need for a case-insensitive
search
* The partial unique index (`users_email_partial_key`) in auth.users is
defined on the condition that `is_sso_user = false`. However, the query
uses `is_sso_user is false` which causes it to not use the index ever
(along with `ilike`). It would be much faster to do `lower(email) = any
(?)` rather than `email ilike any (?)`
* Fixes supabase#1390
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 15, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.139.2](supabase/auth@v2.139.1...v2.139.2)
(2024-02-08)


### Bug Fixes

* improve perf in account linking
([supabase#1394](supabase#1394))
([8eedb95](supabase@8eedb95))
* OIDC provider validation log message
([supabase#1380](supabase#1380))
([27e6b1f](supabase@27e6b1f))
* only create or update the email / phone identity after it's been
verified ([supabase#1403](supabase#1403))
([2d20729](supabase@2d20729))
* only create or update the email / phone identity after it's been
verified (again)
([supabase#1409](supabase#1409))
([bc6a5b8](supabase@bc6a5b8))
* unmarshal is_private_email correctly
([supabase#1402](supabase#1402))
([47df151](supabase@47df151))
* use `pattern` for semver docker image tags
([supabase#1411](supabase#1411))
([14a3aeb](supabase@14a3aeb))


### Reverts

* "fix: only create or update the email / phone identity after i…
([supabase#1407](supabase#1407))
([ff86849](supabase@ff86849))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

Account linking queries do a sequential scan on the users/identities table by default in managed Supabase
4 participants