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

Add Upper index for email in EmailAddress model #3382

Merged
merged 4 commits into from
Aug 25, 2023

Conversation

varunsaral
Copy link
Contributor

This PR add index for email field in EmailAddress Model, this is to improve email__iexact filter in function filter_users_by_email

@varunsaral
Copy link
Contributor Author

This PR fixes #3129

@@ -41,6 +43,7 @@ class Meta:
condition=Q(verified=True),
)
]
indexes = [Index(Upper("email"), name="idx_upper_email")]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other indexes are named like this:

Indexes:
    "account_emailaddress_pkey" PRIMARY KEY, btree (id)

Could you rename this one to account_emailaddress_upper_email ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have renamed the index and manually renamed in migration file, i hope that's the correct way instead of deleting and regenerating the migration file.

@pennersr
Copy link
Owner

One final thing, can you make sure the code is black formatted? The build is failing on that now. Thanks.

@varunsaral
Copy link
Contributor Author

I did the black formatting, sorry for the inconvenience.

@coveralls
Copy link

coveralls commented Aug 25, 2023

Coverage Status

coverage: 91.321% (+0.02%) from 91.304% when pulling 5e71caa on varunsaral:create_email_upper_index into c52304f on pennersr:main.

@varunsaral
Copy link
Contributor Author

applied isort as suggested by tests

@pennersr pennersr merged commit b072cca into pennersr:main Aug 25, 2023
20 checks passed
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.

3 participants