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): /places entries sometimes not ordered alphabetically #10514

Merged
merged 1 commit into from
Jun 21, 2024

Conversation

mertalev
Copy link
Contributor

Description

Whether this query returns a sorted result depends on the query plan. This PR makes it consistently sort them before returning.

How Has This Been Tested?

Tested on an instance where the results were not sorted and confirmed that the revised query sorts them as expected.

@mertalev mertalev merged commit 42f3b50 into main Jun 21, 2024
22 checks passed
@mertalev mertalev deleted the fix/server-get-cities branch June 21, 2024 03:48
@pyorot
Copy link
Contributor

pyorot commented Jun 21, 2024

@mertalev Is alphabet sort what we want here? Could it be easier for users to "explore" if it worked similarly to /people, that is, the user doesn't have a idea of who ey might find in advance, and so is most likely to find the most useful people by having them sorted by number of photos they appear in. So sorting places by number of contained assets comes to mind. Down the line, there could be clustering done, sorting by distance from a centre and making nearby places contiguous.

Lmk if I should make this a discussion; cheers.

@mertalev
Copy link
Contributor Author

mertalev commented Jun 21, 2024

You can check if there's an existing feature request to change it and make one if not.

I think the challenge is that counting etc. makes the query (and hence the page) much slower. With >1 million assets, a naive query can take seconds to run, whereas right now it still loads instantly.

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.

None yet

3 participants