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

additional database indexes as per TN-3308 #4396

Merged
merged 13 commits into from
Aug 9, 2024
Merged

Conversation

pbugni
Copy link
Collaborator

@pbugni pbugni commented Aug 7, 2024

Using the table in TN-3308 as a guide, pencil traced the code to find the common queries on each respective table.

I'd then assemble a query using flask shell to get the exact SQLAlchemy syntax, and then attempt before and after explain $query times, to confirm it would be of value.

Note the few in the migration that are directly added via SQL exec as the SQLAlchemy syntax didn't support (at least I couldn't find a way).

@pbugni pbugni requested a review from ivan-c August 7, 2024 21:06
@pep8speaks
Copy link

pep8speaks commented Aug 7, 2024

Hello @pbugni! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 22:101: E501 line too long (133 > 100 characters)
Line 25:101: E501 line too long (105 > 100 characters)
Line 26:101: E501 line too long (115 > 100 characters)
Line 29:101: E501 line too long (127 > 100 characters)
Line 30:101: E501 line too long (123 > 100 characters)
Line 32:101: E501 line too long (107 > 100 characters)
Line 33:101: E501 line too long (119 > 100 characters)
Line 34:101: E501 line too long (105 > 100 characters)
Line 37:101: E501 line too long (132 > 100 characters)
Line 38:101: E501 line too long (133 > 100 characters)
Line 39:101: E501 line too long (112 > 100 characters)
Line 47:101: E501 line too long (102 > 100 characters)
Line 48:101: E501 line too long (104 > 100 characters)

Comment last updated at 2024-08-08 23:54:42 UTC

@pbugni pbugni requested a review from mcjustin August 7, 2024 21:06
Copy link
Member

@ivan-c ivan-c left a comment

Choose a reason for hiding this comment

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

looks good, thanks!

I re-ran the seq_vs_idx query again and most of the highest-ranked offenders are dealt with in this PR, except user_observations, which is now #1

         relname         | seq_vs_idx | seq_scan | idx_scan  | rel_size  |     hint      
-------------------------+------------+----------+-----------+-----------+---------------
 user_observations       |   12243365 | 12243365 |         0 |    663552 | Check indexes
 questionnaire_responses |    5651062 |  5950648 |    299586 | 130998272 | Check indexes
 encounter_codings       |    1907006 |  1907006 |         0 |   4161536 | Check indexes
 encounters              |    1586471 |  3540200 |   1953729 |  88932352 | Check indexes
 tou                     |      49417 |    49417 |         0 |   2260992 | Check indexes

@pbugni
Copy link
Collaborator Author

pbugni commented Aug 8, 2024

Thank you @ivan-c , great catch on the user_observations table.

@pbugni pbugni changed the base branch from develop to master August 9, 2024 00:41
@pbugni pbugni merged commit ed62a1d into master Aug 9, 2024
2 checks passed
@pbugni pbugni deleted the feature/additional-indexes branch August 9, 2024 00:43
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.

4 participants