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

order: metatags aren't indexed. #3000

Closed
evazion opened this issue Apr 25, 2017 · 3 comments
Closed

order: metatags aren't indexed. #3000

evazion opened this issue Apr 25, 2017 · 3 comments

Comments

@evazion
Copy link
Member

evazion commented Apr 25, 2017

These order: metatags are prone to timing out for Members:

  • order:change
  • order:comment
  • order:comment_bumped
  • order:note
  • order:mpixels
  • order:portrait
  • order:landscape

The problem is these things order by posts.field desc, posts.id desc, but the indexes are all single column. Either the indexes need to be multicolumn, or the queries need to drop the secondary posts.id desc order clause.

Timestamp based orderings could probably get away with dropping posts.id desc. The ordering would be unstable for two posts updated at the same time, but in practice that's probably rare.

For orderings based on width/height or filesize, collisions are probably more common so multicolumn indexes may be more necessary.

@r888888888
Copy link
Collaborator

For now I'll remove the secondary sort and see how it impacts searches in practice.

r888888888 added a commit that referenced this issue Apr 26, 2017
@Type-kun
Copy link
Collaborator

Uh, for score and favcount ordering must have secondary sorting, otherwise the order would be essentially random for images with same scores. This is worse than it sounds since it breaks pagination; we've already encountered this for comment ordering by score. Same goes for dimensional sorting, but it's less often used.

@evazion
Copy link
Member Author

evazion commented Apr 28, 2017

score and favcount don't have indexes either, so the secondary sort doesn't hurt them anyway. In fact, on second look, order:change, order:comment, order:portrait, and order:landscape aren't indexed at all either.

So last_comment_bumped_at and last_noted_at are the only things that really benefit from dropping the secondary sort. But it looks like these indexes still aren't being used, because they're defined by default as ASC NULLS LAST but the sort is doing DESC NULLS LAST.

r888888888 added a commit that referenced this issue May 22, 2017
Fix #3000: order: metatags aren't indexed.
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

No branches or pull requests

3 participants