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 indexes on user+id to changeset and diary comments #5531

Merged
merged 1 commit into from
Jan 21, 2025

Conversation

tomhughes
Copy link
Member

This adds indexes on user+id to diary and changeset comments to make comment views more efficient.

Once this is merged we can switch the diary_comments association on users to sort by id instead of creation date and then drop the creation date indexes.

@tomhughes tomhughes force-pushed the user-comment-indexes branch from 3ebe31d to 0100c2c Compare January 21, 2025 20:13
@tomhughes tomhughes force-pushed the user-comment-indexes branch from 0100c2c to ebe7cf8 Compare January 21, 2025 20:16
@tomhughes tomhughes changed the title Add indes on user+id to changeset and diary comments Add indexes on user+id to changeset and diary comments Jan 21, 2025
@AntonKhorev
Copy link
Collaborator

Creation date indexes might still be useful for combining pagination with date pickers #5255 (comment)

@tomhughes
Copy link
Member Author

Well then you'd ideally want an index on author+date+id though we have to think about how much space we want o use on indexes at some point - is here really a need for filtering by date? How often do you know that you need to look at comments from a particular date?

@tomhughes
Copy link
Member Author

Those indexes are all pretty tiny currently so I guess keeping them doesn't hurt.

@AntonKhorev
Copy link
Collaborator

filtering by date

Not filtering really, just get the starting id by date and then use it as before or after.

@AntonKhorev AntonKhorev merged commit 7b19ba5 into openstreetmap:master Jan 21, 2025
22 checks passed
@tomhughes tomhughes deleted the user-comment-indexes branch January 21, 2025 22:58
@AntonKhorev
Copy link
Collaborator

an index on author+date+id

+id is probably not necessary. If a user managed to create multiple objects during the same microsecond we can afford picking a wrong one. Or select min/max(id) where created_at is between target_date and target_date+/-epsilon.

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.

2 participants