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

Users table index on recovery_token is not being used in queries #1398

Closed
2 tasks done
travis-humata opened this issue Feb 5, 2024 · 2 comments
Closed
2 tasks done
Labels
bug Something isn't working

Comments

@travis-humata
Copy link

Bug report

The following query, which should be nearly instant, accounts for 8% of our DB usage and 1500 ms per query:

SELECT (a bunch of columns) 
FROM users AS users 
WHERE recovery_token = $1 and is_sso_user = $2 
LIMIT $3

On investigation this query is running a seq_scan instead of using the index on recovery_token:

Screenshot 2024-02-05 at 11 43 10 AM

This is the index in question:

CREATE UNIQUE INDEX recovery_token_idx ON auth.users USING btree (recovery_token) WHERE ((recovery_token)::text !~ '^[0-9 ]*$'::text)
  • I confirm this is a bug with Supabase, not with my own application.
  • I confirm I have searched the Docs, GitHub Discussions, and Discord.

Describe the bug

A clear and concise description of what the bug is.

To Reproduce

Run:

explain analyze SELECT *
FROM auth.users
WHERE recovery_token = 'abc';

Expected behavior

Should be nearly instant and use index in query plan

@travis-humata travis-humata added the bug Something isn't working label Feb 5, 2024
J0 added a commit that referenced this issue Mar 4, 2024
…ueries (#1454)

## What kind of change does this PR introduce?

Use the nil instance ID so that we can leverage the compound index on
instance_id and user_id when performing search and deletion


Aims to address #1449 and #1398. 

We note that `LogoutAllRefreshTokens` was marked as deprecated and may
be possible to remove if all access tokens now have a `sessionId`.
Additionally, `FindTokenBySessionID` can likely be replaced by using
`FindCurrentlyActiveRefreshToken`.

We will revisit these in a week or two but they are out of scope for
this PR.

Co-authored-by: joel <[email protected]>
@J0
Copy link
Contributor

J0 commented Mar 12, 2024

closing as this has been deployed - let us know if there are still issues though

@J0 J0 closed this as completed Mar 12, 2024
@nthouliss
Copy link

nthouliss commented May 22, 2024

Hi,

I'm still seeing this issue on my project. Is the index supposed to one provisioned by supabase or should we be creating this index ourselves? I'd just like to verify this because I can see that no index is created for the column on my project.

Edit: Quick correction, there is an index on my project for recovery_token on auth.users but it isn't being used.

recovery_token_no_index

uxodb pushed a commit to uxodb/auth that referenced this issue Nov 13, 2024
…ueries (supabase#1454)

## What kind of change does this PR introduce?

Use the nil instance ID so that we can leverage the compound index on
instance_id and user_id when performing search and deletion


Aims to address supabase#1449 and supabase#1398. 

We note that `LogoutAllRefreshTokens` was marked as deprecated and may
be possible to remove if all access tokens now have a `sessionId`.
Additionally, `FindTokenBySessionID` can likely be replaced by using
`FindCurrentlyActiveRefreshToken`.

We will revisit these in a week or two but they are out of scope for
this PR.

Co-authored-by: joel <[email protected]>
LashaJini pushed a commit to LashaJini/auth that referenced this issue Nov 13, 2024
…ueries (supabase#1454)

## What kind of change does this PR introduce?

Use the nil instance ID so that we can leverage the compound index on
instance_id and user_id when performing search and deletion


Aims to address supabase#1449 and supabase#1398. 

We note that `LogoutAllRefreshTokens` was marked as deprecated and may
be possible to remove if all access tokens now have a `sessionId`.
Additionally, `FindTokenBySessionID` can likely be replaced by using
`FindCurrentlyActiveRefreshToken`.

We will revisit these in a week or two but they are out of scope for
this PR.

Co-authored-by: joel <[email protected]>
LashaJini pushed a commit to LashaJini/auth that referenced this issue Nov 15, 2024
…ueries (supabase#1454)

## What kind of change does this PR introduce?

Use the nil instance ID so that we can leverage the compound index on
instance_id and user_id when performing search and deletion


Aims to address supabase#1449 and supabase#1398. 

We note that `LogoutAllRefreshTokens` was marked as deprecated and may
be possible to remove if all access tokens now have a `sessionId`.
Additionally, `FindTokenBySessionID` can likely be replaced by using
`FindCurrentlyActiveRefreshToken`.

We will revisit these in a week or two but they are out of scope for
this PR.

Co-authored-by: joel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants