fix: filter null duplicate assets

[ItzDerock]

Description

Fixes #18884

On my Immich instance, trying to use the /utilities/duplicates menu resulted in an internal server error:

TypeError: Cannot read properties of null (reading 'id')
    at mapAsset (/usr/src/app/server/dist/dtos/asset-response.dto.js:186:20)
    at /usr/src/app/server/dist/services/duplicate.service.js:25:77
    at Array.map (<anonymous>)
    at /usr/src/app/server/dist/services/duplicate.service.js:25:28
    at Array.map (<anonymous>)
    at DuplicateService.getDuplicates (/usr/src/app/server/dist/services/duplicate.service.js:23:27)
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5)

How Has This Been Tested?

• Tested functionality on a fresh Immich installation
• Built the prod container and ran it in production, verifying that /utilities/duplicates worked now.

Screenshots (if appropriate)

Same error screen as #18884

Checklist:

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation if applicable
• I have no unrelated changes in the PR.
• I have confirmed that any new dependencies are strictly necessary.
• I have written tests for new code (if applicable)
• I have followed naming conventions/patterns in the surrounding code
• All code in src/services/ uses repositories implementations for database calls, filesystem operations, etc.
• All code in src/repositories/ is pretty basic/simple and does not have any immich specific logic (that belongs in src/services/)

other notes

Not sure if a test is applicable here, would be nice to prevent regressions, but I wasn't able to cleanly replicate the bug in a test environment.

I believe it's related to an asset not having a corresponding asset_exif entry, but I've tried uploading duplicates stripped of exif data and that didn't reproduce the error. My production instance is >1tb, so trying to debug that didn't seem feasible. So, in the end, I just added a null check.

[mertalev]

Does changing the leftJoinLateral above it to innerJoinLateral also fix it? I think it'd be better than this change.

[ItzDerock]

Changing to innerJoinLateral does fix it, though I wonder if doing an inner join or my original solution means there are some duplicates that will end up being filtered out and never shown to the user.

[ItzDerock]

edit: nvm this doesn't quite work

I've also tried slightly rewriting the query to use a non-lateral left join,

which seems to work fine on my local machine and when pushed to my larger immich instance. Both this change and the simple swap to innerJoinLateral yield 554 duplicates in my dataset, so not sure if this new query is any better or worse. Casting to MapAsset wasn't needed in the new query, so theoretically should be more type-safe..?

lmk what you think

[mertalev]

The query you posted doesn't include the exif columns, does it? That's why the lateral join exists.

[ItzDerock]

yeah, i realized. I've got this now:

realized that the concat operator works with json objects, so I can combine without using a lateral join. Works locally, testing on my other instance now.

[mertalev]

Nice approach! I think that one would return the same result as the inner join lateral. Unfortunately, I think it'd be slower.

[ItzDerock]

Yeah I think you're right. Both return 554 entries on my instance, and all that json conversions have a non-zero impact. The thought was that an innerJoinLateral might omit perfectly valid duplicate assets that were missing an entry on asset_exif, while a leftJoin will find duplicates and include exif if it exists otherwise set exif data to null. But, I'm not very familiar with this codebase so maybe my initial assumption of the issue arising from non-existing asset_exif is wrong.

[mertalev]

It's an edge case. The expectation is that there's a row in the exif table, since it's an earlier step before embeddings are generated etc. The row would exist after metadata extraction succeeds regardless of how much metadata the asset has. But it's technically possible for the row to be missing, so it's fair to address that.

[mertalev]

You should run make sql with a running dev instance to update the generated SQL file.