fix(server): prevent duplicate geodata temp table

[mertalev]

Description

It seems it's possible for the tmp table to exist in some cases, possibly when upgrading from older versions. This PR ensures the table is dropped before continuing.

Also moves one of the indices to be created before ingestion as it is noticeably faster in this case.

[bo0tzz]

this fillfactor param is lost in this PR, does that matter?

[mertalev]

It's intentionally removed because it only makes sense to have it if there are no inserts to the index after its creation

[skatsubo]

Also WITH (fillfactor = 100) can be omitted because it's the default in all Postgres versions.

https://www.postgresql.org/docs/current/sql-createtable.html#RELOPTION-FILLFACTOR

100 (complete packing) is the default.

[mertalev]

The table fill factor is 100, but the index fill factor is lower. Setting the index fill factor to 100 only makes sense if the index data is completely static.

[skatsubo]

Gotcha. Thanks for the correction!
Proper link instead of what I posted before: https://www.postgresql.org/docs/current/sql-createindex.html#INDEX-RELOPTION-FILLFACTOR

B-trees use a default fillfactor of 90

[zackpollard]

I'll take a look at this on Tuesday. I'm sure I made a migration to drop the tmp tables if they existed.

[zackpollard]

This should be impossible

immich/server/src/migrations/1743611339000-GeodataCleanup.ts

Line 10 in 2248a38

await queryRunner.query(`DROP TABLE IF EXISTS "geodata_places_tmp"`);

[skatsubo]

This should be impossible

immich/server/src/migrations/1743611339000-GeodataCleanup.ts

Line 10 in 2248a38

await queryRunner.query(`DROP TABLE IF EXISTS "geodata_places_tmp"`);

#17914
Looks like flip-flopping 1.132.1 -> 1.131.3 -> 1.132.1 + interrupting container on 1.131.3 while in geodata loading => caused this behavior:

• lingering _tmp table after 1.131.3
• no cleanup of _tmp afterwards (on the second 1.132.1) because the cleanup migration was already applied on the first 1.132.1

[zackpollard]

This should be impossible

immich/server/src/migrations/1743611339000-GeodataCleanup.ts

Line 10 in 2248a38

await queryRunner.query(`DROP TABLE IF EXISTS "geodata_places_tmp"`);

#17914 Looks like flip-flopping 1.132.1 -> 1.131.3 -> 1.132.1 + interrupting container on 1.131.3 while in geodata loading => caused this behavior:

* lingering `_tmp` table after 1.131.3

* no cleanup of `_tmp` afterwards (on the second 1.132.1) because the cleanup migration was already applied on the first 1.132.1

Interesting observation, perhaps we should specifically block downgrades after migrations are applied without also rolling back your database 🤔