immich
immich copied to clipboard
refactor(server): use kysely
Description
This PR transitions the asset and search repositories from TypeORM to Kysely. The goals of this change include type safety—including for complex queries—improved performance, more transparent behavior and better code composability. Other repositories as well as dev tooling will be migrated in later PRs to limit the scope of this one.
Notes:
- Almost every query is faster than before, some significantly so. However, there is a possible regression for queries using both file and tag relations, in the order of single-digit milliseconds for 100 assets. This seems to be because the plan for the subquery approach is more reliant on random access through indices. This is only measuring the queries themselves — the difference may or may not disappear after accounting for the additional postprocessing that TypeORM needs to do.
- Most repository methods have the same signatures as before to ease the transition. There are some exceptions to this:
- Some upsert/update methods have Kysely types as inputs since this was an easy change and made things simpler
- I made changes to some queries that necessitated a different signature
- I noticed that the
includeNullparameter wasn't being respected for the search suggestion queries, so I changed their signatures to handle it
- Kysely considers ORM-like upsert helpers to be out of scope. Since this is a common need, I made a few type-safe helpers to make these queries more ergonomic. A related limitation is that there's no reference to the columns at runtime, so the server introspects the database after migrations run to get this information (TypeORM provides this as well, but I didn't want to rely on it).
- There can be differences in JSON-ified and normal DB responses.
byteatypes are returned as hex-encoded strings in the former and asBufferin the latter. Timestamps likewise end with+00:00in JSON and.000Zwithout.+00:00is more accurate if anything, but the inconsistency is a bit annoying.- I'm not sure if it's worth parsing the hex strings into buffers and back into base64 strings. The payload is smaller this way, but the server spends more time to do this.
Testing
Over the course of working on this branch, I've tested almost everything the web app can do at one point or another. But there may be regressions since testing certain functionality or subtle issues that I haven't noticed, and I haven't tested it with the mobile app. The one issue I'm currently seeing is that scrubbing the timeline doesn't work: I can't move it, even though normal scrolling works.
If testing this PR, be sure to make the indices mentioned in the TODO comments in the asset repository. I haven't made a migration to apply them yet.
Will this fix #11868?
Will this fix #11868?
Yup.
Nice! Any changes here that would affect people using external Postgres? I don’t see any new extensions etc.
Nice! Any changes here that would affect people using external Postgres? I don’t see any new extensions etc.
I don't think so. It's an internal change
I just performed a functional testing run.
- The
Tagpage doesn't render the assets correctly - The sync endpoint throws the error below
immich_server | [Nest] 28 - 12/20/2024, 4:06:42 PM ERROR [Api:ErrorInterceptor~2thpecne] Unknown error: TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
immich_server | TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
immich_server | at extname (node:path:1463:5)
immich_server | at Object.lookup (/usr/src/app/src/utils/mime-types.ts:90:51)
immich_server | at mapAsset (/usr/src/app/src/dtos/asset-response.dto.ts:140:33)
immich_server | at <anonymous> (/usr/src/app/src/services/sync.service.ts:24:38)
immich_server | at Array.map (<anonymous>)
immich_server | at SyncService.getFullSync (/usr/src/app/src/services/sync.service.ts:24:19)
immich_server | at process.processTicksAndRejections (node:internal/process/task_queues:105:5)
Hi Mert, the latest update causes the web timeline to be unable to load
Hi Mert, the latest update causes the web timeline to be unable to load
I haven't been able to reproduce this.
Hi Mert, the latest update causes the web timeline to be unable to load
I haven't been able to reproduce this.
I am running the latest update with make dev-update and this is what I see, same behavior for album, and in incognito
Is it possible that this change broke the DuplicatService?
See my Q&A: https://github.com/immich-app/immich/discussions/15722