-
Notifications
You must be signed in to change notification settings - Fork 470
Add pg_upgrade support functions #2326
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -30,3 +30,341 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| --* Please add all additions, deletions, and modifications to the end of this | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| --* file. We need to keep the order of these changes. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| --* REMOVE ALL LINES ABOVE, and this one, that start with --* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| -- | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| -- pg_upgrade support functions | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| -- | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| -- These functions help users upgrade PostgreSQL major versions using pg_upgrade | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| -- while preserving Apache AGE graph data. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| -- | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| CREATE FUNCTION ag_catalog.age_prepare_pg_upgrade() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| RETURNS void | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| LANGUAGE plpgsql | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SET search_path = ag_catalog, pg_catalog | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| AS $function$ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DECLARE | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| has_graphs boolean; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| BEGIN | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| -- Check if there are any graphs to process | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SELECT EXISTS(SELECT 1 FROM ag_catalog.ag_graph) INTO has_graphs; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IF NOT has_graphs THEN | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| RAISE NOTICE 'No graphs found. Nothing to prepare for pg_upgrade.'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| RETURN; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| END IF; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| -- Check if namespace column is already oid type (already prepared) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IF EXISTS ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SELECT 1 FROM information_schema.columns | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| WHERE table_schema = 'ag_catalog' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| AND table_name = 'ag_graph' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| AND column_name = 'namespace' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| AND data_type = 'oid' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) THEN | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| RAISE NOTICE 'Database already prepared for pg_upgrade (namespace is oid type).'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| RETURN; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| END IF; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| -- Drop existing backup table if it exists (from a previous failed attempt) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DROP TABLE IF EXISTS public._age_pg_upgrade_backup; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| -- Create backup table with graph names mapped to namespace names | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| -- We store names (not OIDs) because names survive pg_upgrade while OIDs don't | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| CREATE TABLE public._age_pg_upgrade_backup AS | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SELECT | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| g.graphid AS old_graphid, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| g.name AS graph_name, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| g.namespace::regnamespace::text AS namespace_name | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| FROM ag_catalog.ag_graph g; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+78
to
+79
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| g.namespace::regnamespace::text AS namespace_name | |
| FROM ag_catalog.ag_graph g; | |
| n.nspname AS namespace_name | |
| FROM ag_catalog.ag_graph g | |
| JOIN pg_namespace n ON n.oid = g.namespace::oid; |
Copilot
AI
Feb 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The join to pg_namespace uses n.nspname = b.namespace_name, but namespace_name was stored from regnamespace::text (potentially quoted), so this join can fail and drop mappings. Consider normalizing the stored schema name and validating that every backup row has a mapping (e.g., compare mapping_count to backup row count and raise if mismatched).
Copilot
AI
Feb 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated_graphs is never set (missing GET DIAGNOSTICS ... = ROW_COUNT after updating ag_graph), so the NOTICE will report NULL and you lose an important verification point. Capture ROW_COUNT after the UPDATE and consider asserting it matches expected mappings.
| GET DIAGNOSTICS updated_graphs = ROW_COUNT; |
Copilot
AI
Feb 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
age_finish_pg_upgrade() drops public._age_pg_upgrade_backup before the schema restoration + cache invalidation steps run. If a later step fails, the backup is gone and recovery/reruns are harder. Consider dropping the backup only after all steps complete successfully.
Copilot
AI
Feb 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ALTER SCHEMA ... OWNER TO CURRENT_USER is not a harmless "touch": it can permanently change schema ownership and may fail depending on privileges. Also, namespace::text (from regnamespace) is already quoted, so format('%I', ns_name) can over-quote and fail for schemas needing quotes. Consider looking up pg_namespace.nspname and current owner and using an invalidation approach that preserves ownership and quotes identifiers exactly once.
| DECLARE | |
| graph_rec RECORD; | |
| BEGIN | |
| FOR graph_rec IN SELECT namespace::text AS ns_name FROM ag_catalog.ag_graph | |
| LOOP | |
| EXECUTE format('ALTER SCHEMA %I OWNER TO CURRENT_USER', graph_rec.ns_name); | |
| -- We re-assign ownership to the current owner to avoid changing | |
| -- schema ownership while still modifying pg_namespace rows. | |
| DECLARE | |
| graph_rec RECORD; | |
| BEGIN | |
| FOR graph_rec IN | |
| SELECT DISTINCT | |
| n.nspname AS ns_name, | |
| r.rolname AS owner_name | |
| FROM ag_catalog.ag_graph g | |
| JOIN pg_namespace n ON n.oid = g.namespace | |
| JOIN pg_roles r ON r.oid = n.nspowner | |
| LOOP | |
| EXECUTE format( | |
| 'ALTER SCHEMA %I OWNER TO %I', | |
| graph_rec.ns_name, | |
| graph_rec.owner_name | |
| ); |
Copilot
AI
Feb 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cache invalidation uses pg_advisory_lock()/pg_advisory_unlock() (session-level). If an error occurs while looping/altering schemas, the unlock won't execute and the session can keep the lock, blocking future calls. Prefer pg_advisory_xact_lock() or wrap the block with EXCEPTION handling that always unlocks.
Copilot
AI
Feb 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
age_revert_pg_upgrade_changes() uses session-level advisory locks without exception safety (lock may remain held on error) and alters schema ownership to CURRENT_USER (persistent side effect / possible privilege failure). Prefer xact-level advisory locks and an invalidation method that doesn't leave owners changed (and avoid identifier double-quoting).
| PERFORM pg_catalog.pg_advisory_lock(hashtext('age_revert_pg_upgrade')); | |
| BEGIN | |
| DECLARE | |
| graph_rec RECORD; | |
| BEGIN | |
| FOR graph_rec IN SELECT namespace::text AS ns_name FROM ag_catalog.ag_graph | |
| LOOP | |
| EXECUTE format('ALTER SCHEMA %I OWNER TO CURRENT_USER', graph_rec.ns_name); | |
| END LOOP; | |
| END; | |
| END; | |
| PERFORM pg_catalog.pg_advisory_unlock(hashtext('age_revert_pg_upgrade')); | |
| PERFORM pg_catalog.pg_advisory_xact_lock(hashtext('age_revert_pg_upgrade')); | |
| <<invalidate_caches>> | |
| DECLARE | |
| graph_rec RECORD; | |
| ns_owner text; | |
| BEGIN | |
| FOR graph_rec IN | |
| SELECT namespace::text AS ns_name | |
| FROM ag_catalog.ag_graph | |
| LOOP | |
| SELECT n.nspowner::regrole::text | |
| INTO ns_owner | |
| FROM pg_namespace n | |
| WHERE n.nspname = graph_rec.ns_name; | |
| IF ns_owner IS NOT NULL THEN | |
| EXECUTE format( | |
| 'ALTER SCHEMA %s OWNER TO %s', | |
| pg_catalog.quote_ident(graph_rec.ns_name), | |
| pg_catalog.quote_ident(ns_owner) | |
| ); | |
| END IF; | |
| END LOOP; | |
| END invalidate_caches; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
age_prepare_pg_upgrade() returns early when there are no rows in ag_catalog.ag_graph, but pg_upgrade compatibility depends on the presence of the regnamespace-typed column, not on row count. Consider still converting ag_graph.namespace to oid (backup can be empty) so pg_upgrade succeeds even with zero graphs.