Skip to content

fix(server): filter parameters by database and role #19392

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

Merged
merged 1 commit into from
Jun 23, 2025

Conversation

mertalev
Copy link
Member

@mertalev mertalev commented Jun 20, 2025

If the Postgres instance has multiple database besides immich and one of them has a database or role wide parameter override set, it is possible for the migration to apply the wrong parameters. Immich happens to work without the right parameters set unless Postgres is also using pgvecto.rs, and it's unlikely for the incorrect parameters to be disruptive. But it's of course not an ideal scenario. This PR filters the query fetching these parameters to avoid this happening.

Fixes #19391

@UliTheGrey
Copy link

Sorry for interfering... while your at this function, wouldn´t

export async function up(qb: Kysely<any>): Promise<void> {
  const [{ db }] =
    (await sql<{ db: string }>`select current_database() as db`.execute(qb)).rows ?? [];

  if (!db) return;

  await sql.raw(`ALTER DATABASE "${db}" RESET vchordrq.prewarm_dim;`).execute(qb);
}

do the trick ?

@mertalev
Copy link
Member Author

Sorry for interfering... while your at this function, wouldn´t

export async function up(qb: Kysely<any>): Promise<void> {
  const [{ db }] =
    (await sql<{ db: string }>`select current_database() as db`.execute(qb)).rows ?? [];

  if (!db) return;

  await sql.raw(`ALTER DATABASE "${db}" RESET vchordrq.prewarm_dim;`).execute(qb);
}

do the trick ?

Unfortunately not. Postgres 15 and later doesn't allow setting or resetting a parameter with a reserved prefix unless that parameter is recognized by an extension. Since the extension no longer uses this parameter, Postgres doesn't let us unset it either. This is a roundabout way to reset it as a result.

@mmomjian
Copy link
Collaborator

will fix #19391

Copy link
Collaborator

@mmomjian mmomjian left a comment

Choose a reason for hiding this comment

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

How confident can we be that this always returns only one row?

I guess if we are scoping for (current DB) AND (global roles) they SHOULD all be contained in one row, but I am not 100% sure of this.

EDIT: I think there is only one row per DB+role combo: https://www.postgresql.org/docs/current/catalog-pg-db-role-setting.html

@mmomjian
Copy link
Collaborator

mmomjian commented Jun 22, 2025

Should key and value be quoted, to be safe? #19436 has some weird DB params that tbh I have no idea how they got set, but in theory other params could contain reserved characters.

await sql.raw(`alter database "${db}" set ${key} to ${value};`).execute(qb);

@zackpollard zackpollard merged commit 0396614 into main Jun 23, 2025
56 of 57 checks passed
@zackpollard zackpollard deleted the fix/server-filter-db-in-migration branch June 23, 2025 11:10
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.

DB params from other roles or DBs are applied to the Immich DB
6 participants