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

refactor(server): use includeNull in query for search suggestions #14626

Merged
merged 10 commits into from
Dec 10, 2024

Conversation

mertalev
Copy link
Contributor

Description

Extracted out from the Kysely branch, this PR refactors the search suggestion methods to use the includeNull option and empty string filter into the queries themselves and changes the queries to use inner joins.

Copy link
Member

@danieldietzler danieldietzler left a comment

Choose a reason for hiding this comment

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

LGTM (minus the broken e2e tests :D)

@mertalev
Copy link
Contributor Author

I think the e2e only worked because there were assets without entries in the exif table at the time of the test. The inner join would mean null isn't one of the options in that case. Since the inner join is good to have regardless, I changed it to just add null to the end if includeNull is true. It does make the flag kind of pointless though lol

@mertalev mertalev merged commit 25ca3b1 into main Dec 10, 2024
36 checks passed
@mertalev mertalev deleted the fix/server-search-suggestions branch December 10, 2024 21:22
yosit pushed a commit to yosit/immich that referenced this pull request Dec 20, 2024
…mmich-app#14626)

* use `includeNull`

* push down `includeNull` into query, inner joins

* remove filter

* update sql

* fix tests

* maybe fix e2e

* more e2e tests

* handle no exif row

* whoops

* update sql
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.

2 participants