From 1b141d5ca9d68b7224f6c85d8dbdf3012cfc1c56 Mon Sep 17 00:00:00 2001
From: David Wolff <david@dav.dev>
Date: Fri, 31 Jan 2025 16:06:45 +0100
Subject: [PATCH] refactor(server): filter assets by people using a subquery
 instead of a cte (#15768)

---
 server/src/entities/asset.entity.ts         | 31 ++++++++++-----------
 server/src/repositories/asset.repository.ts | 13 ++++-----
 2 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/server/src/entities/asset.entity.ts b/server/src/entities/asset.entity.ts
index e9dbe67a2fffc..879c2c51699cd 100644
--- a/server/src/entities/asset.entity.ts
+++ b/server/src/entities/asset.entity.ts
@@ -238,24 +238,20 @@ export function withFacesAndPeople(eb: ExpressionBuilder<DB, 'assets'>) {
     .as('faces');
 }
 
-/** Adds a `has_people` CTE that can be inner joined on to filter out assets */
-export function hasPeopleCte(db: Kysely<DB>, personIds: string[]) {
-  return db.with('has_people', (qb) =>
-    qb
-      .selectFrom('asset_faces')
-      .select('assetId')
-      .where('personId', '=', anyUuid(personIds!))
-      .groupBy('assetId')
-      .having((eb) => eb.fn.count('personId').distinct(), '=', personIds.length),
+export function hasPeople<O>(qb: SelectQueryBuilder<DB, 'assets', O>, personIds: string[]) {
+  return qb.innerJoin(
+    (eb) =>
+      eb
+        .selectFrom('asset_faces')
+        .select('assetId')
+        .where('personId', '=', anyUuid(personIds!))
+        .groupBy('assetId')
+        .having((eb) => eb.fn.count('personId').distinct(), '=', personIds.length)
+        .as('has_people'),
+    (join) => join.onRef('has_people.assetId', '=', 'assets.id'),
   );
 }
 
-export function hasPeople(db: Kysely<DB>, personIds?: string[]) {
-  return personIds && personIds.length > 0
-    ? hasPeopleCte(db, personIds).selectFrom('assets').innerJoin('has_people', 'has_people.assetId', 'assets.id')
-    : db.selectFrom('assets');
-}
-
 export function withOwner(eb: ExpressionBuilder<DB, 'assets'>) {
   return jsonObjectFrom(eb.selectFrom('users').selectAll().whereRef('users.id', '=', 'assets.ownerId')).as('owner');
 }
@@ -326,8 +322,11 @@ const joinDeduplicationPlugin = new DeduplicateJoinsPlugin();
 export function searchAssetBuilder(kysely: Kysely<DB>, options: AssetSearchBuilderOptions) {
   options.isArchived ??= options.withArchived ? undefined : false;
   options.withDeleted ||= !!(options.trashedAfter || options.trashedBefore);
-  return hasPeople(kysely.withPlugin(joinDeduplicationPlugin), options.personIds)
+  return kysely
+    .withPlugin(joinDeduplicationPlugin)
+    .selectFrom('assets')
     .selectAll('assets')
+    .$if(!!options.personIds && options.personIds.length > 0, (qb) => hasPeople(qb, options.personIds!))
     .$if(!!options.createdBefore, (qb) => qb.where('assets.createdAt', '<=', options.createdBefore!))
     .$if(!!options.createdAfter, (qb) => qb.where('assets.createdAt', '>=', options.createdAfter!))
     .$if(!!options.updatedBefore, (qb) => qb.where('assets.updatedAt', '<=', options.updatedBefore!))
diff --git a/server/src/repositories/asset.repository.ts b/server/src/repositories/asset.repository.ts
index 1f9f8f997f83b..b306b1a694473 100644
--- a/server/src/repositories/asset.repository.ts
+++ b/server/src/repositories/asset.repository.ts
@@ -8,7 +8,6 @@ import { Chunked, ChunkedArray, DummyValue, GenerateSql } from 'src/decorators';
 import {
   AssetEntity,
   hasPeople,
-  hasPeopleCte,
   searchAssetBuilder,
   truncatedDate,
   withAlbums,
@@ -576,7 +575,7 @@ export class AssetRepository implements IAssetRepository {
   @GenerateSql({ params: [{ size: TimeBucketSize.MONTH }] })
   async getTimeBuckets(options: TimeBucketOptions): Promise<TimeBucketItem[]> {
     return (
-      ((options.personId ? hasPeopleCte(this.db, [options.personId]) : this.db) as Kysely<DB>)
+      this.db
         .with('assets', (qb) =>
           qb
             .selectFrom('assets')
@@ -589,11 +588,7 @@ export class AssetRepository implements IAssetRepository {
                 .innerJoin('albums_assets_assets', 'assets.id', 'albums_assets_assets.assetsId')
                 .where('albums_assets_assets.albumsId', '=', asUuid(options.albumId!)),
             )
-            .$if(!!options.personId, (qb) =>
-              qb.innerJoin(sql.table('has_people').as('has_people'), (join) =>
-                join.onRef(sql`has_people."assetId"`, '=', 'assets.id'),
-              ),
-            )
+            .$if(!!options.personId, (qb) => hasPeople(qb, [options.personId!]))
             .$if(!!options.withStacked, (qb) =>
               qb
                 .leftJoin('asset_stack', (join) =>
@@ -628,10 +623,12 @@ export class AssetRepository implements IAssetRepository {
 
   @GenerateSql({ params: [DummyValue.TIME_BUCKET, { size: TimeBucketSize.MONTH, withStacked: true }] })
   async getTimeBucket(timeBucket: string, options: TimeBucketOptions): Promise<AssetEntity[]> {
-    return hasPeople(this.db, options.personId ? [options.personId] : undefined)
+    return this.db
+      .selectFrom('assets')
       .selectAll('assets')
       .$call(withExif)
       .$if(!!options.albumId, (qb) => withAlbums(qb, { albumId: options.albumId }))
+      .$if(!!options.personId, (qb) => hasPeople(qb, [options.personId!]))
       .$if(!!options.userIds, (qb) => qb.where('assets.ownerId', '=', anyUuid(options.userIds!)))
       .$if(options.isArchived !== undefined, (qb) => qb.where('assets.isArchived', '=', options.isArchived!))
       .$if(options.isFavorite !== undefined, (qb) => qb.where('assets.isFavorite', '=', options.isFavorite!))