Skip to content

Fix hardcoded postgres role on set_updated_at function #306

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.

Sign up for GitHub

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

Open
matingathani wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix hardcoded postgres role on set_updated_at function #306

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/sync-engine/src/database/migrations/0043_fix_set_updated_at_owner.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-- Migration 0012 hardcoded "postgres" as the function owner, which fails on
-- PostgreSQL installations where that role does not exist (e.g. managed cloud
-- databases). Re-assign ownership to the role executing the migration so it
-- works in any environment.
Comment on lines +1 to +4
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

The header comment says this “works in any environment”, but on clusters where role postgres doesn’t exist the failure occurs in migration 0012 and pg-node-migrations stops after rolling back 0012, so 0043 is never reached. Please clarify the comment (and/or PR description) about what scenarios this migration actually fixes vs. which still require a separate remediation path (fresh installs on non-postgres systems).

Copilot uses AI. Check for mistakes.
ALTER FUNCTION set_updated_at() OWNER TO CURRENT_USER;
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

This migration depends on the session search_path to resolve set_updated_at(). If search_path is not consistent across environments (or differs from the one used when 0012 ran), this can fail with function set_updated_at() does not exist. Consider schema-qualifying the function name (e.g., stripe.set_updated_at()), consistent with the rest of the migrations’ explicit stripe.* references, to make the migration independent of connection defaults.

Suggested change
ALTER FUNCTION set_updated_at() OWNER TO CURRENT_USER;
ALTER FUNCTION stripe.set_updated_at() OWNER TO CURRENT_USER;

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

ALTER FUNCTION ... OWNER TO ... will fail unless the migration role is either a superuser or the current owner of set_updated_at(). If 0012 ran as a different role (or set the owner to a role the migrator is not a member of), this migration can abort and stop all subsequent migrations. Consider making this change tolerant to insufficient privileges (e.g., run inside a DO block that catches insufficient_privilege / undefined_function and no-ops) so migrations remain runnable across role changes and partial installs.

Suggested change
ALTER FUNCTION set_updated_at() OWNER TO CURRENT_USER;
DO $$
BEGIN
ALTER FUNCTION set_updated_at() OWNER TO CURRENT_USER;
EXCEPTION
WHEN insufficient_privilege OR undefined_function THEN
-- If we don't own the function or it doesn't exist, ignore and continue.
NULL;
END;
$$;

Copilot uses AI. Check for mistakes.
Loading