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

fix(server): /search/random failing with certain options #13040

Merged
merged 3 commits into from
Sep 30, 2024

Conversation

mertalev
Copy link
Contributor

@mertalev mertalev commented Sep 30, 2024

Description

The current query only works when no relations are requested. When they are, TypeORM does an additional query with a distinct clause that conflicts with the sort order. This PR makes a few changes to address this:

  • Changes the query to not rely on ordering and instead use a where clause leveraging the random nature of UUIDs. This has the additional advantage of making the query scale much better to larger libraries
  • Does two simultaneous queries with opposite constraints: this is to ensure that the requested number of results is provided if enough matching rows exist in the DB. Filtering only in one direction of the random UUID risks excluding too many rows. This can be done more efficiently using UNION ALL, but the fact that TypeORM does two queries makes it infeasible right now
  • Fixes the withPeople option that's currently broken
  • Removes pagination from the /search/random endpoint since it makes things more complicated and there isn't a compelling use-case for it here

Fixes #13021

How Has This Been Tested?

Tested that the endpoint works with a combination of withExif, withStacked and withPeople and returns different rows in each call.

Copy link
Contributor

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

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

Looks much better, thanks!

@mertalev mertalev merged commit 7adb35e into main Sep 30, 2024
36 checks passed
@mertalev mertalev deleted the fix/server-search-random branch September 30, 2024 04:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API: POST /search/random returns internal server error
2 participants