Skip to content

Conversation

sandhose
Copy link
Member

@sandhose sandhose commented Sep 15, 2025

No description provided.

Copy link

cloudflare-workers-and-pages bot commented Sep 15, 2025

Deploying matrix-authentication-service-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: a7e56b3
Status: ✅  Deploy successful!
Preview URL: https://5a0b290c.matrix-authentication-service-docs.pages.dev
Branch Preview URL: https://quenting-admin-api-user-sear.matrix-authentication-service-docs.pages.dev

View logs

-- This enables the pg_trgm extension, which is used for search filters
-- Starting Posgres 16, this extension is marked as "trusted", meaning it can be
-- installed by non-superusers
CREATE EXTENSION IF NOT EXISTS pg_trgm;
Copy link
Member Author

@sandhose sandhose Sep 15, 2025

Choose a reason for hiding this comment

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

The only tricky thing is that before PG 16 you have to be superuser to execute that. Which is probably fine in most Docker environments for example?

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm yikes. Feels like something we should at least call out somewhere, v15 still has another 2 years on it and I'm not sure people will know how to run this manually as a superuser.

@sandhose sandhose requested a review from reivilibre September 15, 2025 12:27
Copy link
Contributor

@reivilibre reivilibre left a comment

Choose a reason for hiding this comment

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

two things to do:

  • add documentation to the database page and upgrade notes for this migration, for Pg <= 15;
  • check that if the superuser on Pg 15 already created the extension, whether a non-superuser is then able to complete the migration as a no-op?

@sandhose
Copy link
Member Author

I will add things in the docs. We don't have upgrade notes, so it will have to be manual in the release notes somehow. I did check for CREATE EXTENSION IF NOT EXISTS with PG 13 and it works:

sandhose=# CREATE ROLE unprivileged;
CREATE ROLE
sandhose=# SET ROLE unprivileged ;
SET
sandhose=> CREATE EXTENSION IF NOT EXISTS pg_trgm;
ERROR:  permission denied to create extension "pg_trgm"
HINT:  Must have CREATE privilege on current database to create this extension.
sandhose=> SET ROLE sandhose ;
SET
sandhose=# CREATE EXTENSION IF NOT EXISTS pg_trgm;
CREATE EXTENSION
sandhose=# SET ROLE unprivileged ;
SET
sandhose=> CREATE EXTENSION IF NOT EXISTS pg_trgm;
NOTICE:  extension "pg_trgm" already exists, skipping
CREATE EXTENSION

@sandhose
Copy link
Member Author

@reivilibre I've added things in the documentation in a3ecdc9 and put what should be added to the release notes in the PR body, wdyt?

@sandhose sandhose requested a review from reivilibre September 15, 2025 13:25
@reivilibre
Copy link
Contributor

release notes mostly OK, would just say that maybe it needs a more obvious 'If you are using Postgres 15 or earlier and MAS doesn't run as a Postgres superuser, run ... as a superuser prior to updating MAS.'

Might just be me but I feel it's easy to miss the fact I might need to do something, in the current text.

@sandhose sandhose force-pushed the quenting/admin-api/user-search branch from a3ecdc9 to a7e56b3 Compare September 16, 2025 10:03
@sandhose
Copy link
Member Author

After double-checking, turns out trusted extensions were a thing introduced in PG 13, which is the version we ask for anyway. So I reverted the docs change

@sandhose sandhose merged commit 8047331 into main Sep 16, 2025
38 checks passed
@sandhose sandhose deleted the quenting/admin-api/user-search branch September 16, 2025 10:22
@sandhose sandhose added A-Admin-API Related to the admin API T-Enhancement New feature of request labels Sep 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Admin-API Related to the admin API T-Enhancement New feature of request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants