[#31938] YSQL: MAGE extension does work in backup/restore scenarios#31981
[#31938] YSQL: MAGE extension does work in backup/restore scenarios#31981shubin-yb wants to merge 4 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request fixes an issue where dropping the mage extension when it is not installed causes an error, which broke backup/restore operations. It adds a check in is_age_drop to verify if the extension is installed before proceeding with the AGE-specific drop path, and includes corresponding regression and integration tests. The reviewer suggested wrapping the test logic in TestYbBackup.java within a try-finally block to ensure robust cleanup of the database and extension in case of test failures, preventing test pollution.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
✅ Deploy Preview for infallible-bardeen-164bc9 ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
shubin-yb
changed the title
sql_dump in binary-upgrade mode emits
DROP EXTENSION IF EXISTS mage;
before recreating an empty extension shell, so any backup/restore of a
database that uses the mage (Apache AGE) extension hits this statement
during restore. The mage ProcessUtility hook intercepts every DROP that
mentions "mage" via is_age_drop() and unconditionally routes it through
drop_age_extension(), which enumerates graphs from mag_catalog.ag_graph.
When the extension is not (yet) installed on the restore target, that
lookup errors out with
ERROR: table "ag_graph" does not exist
and the whole restore aborts, so a backup of a mage-using database was
effectively unrestorable.
Fix: guard is_age_drop() with get_extension_oid("mage", true). When the
extension is not installed, defer to standard ProcessUtility, which
correctly performs the IF EXISTS no-op (or raises the standard
"extension does not exist" error for a bare DROP EXTENSION mage).
get_extension_oid is preferred over checking the mag_catalog schema:
the schema can linger after a prior DROP EXTENSION (so a schema-based
guard still ran the AGE drop path and still hit the ag_graph lookup).
Tests:
* src/postgres/third-party-extensions/mage/regress/{sql,expected}/
yb.orig.basic.{sql,out}: three direct regress cases appended at the
end of the existing yb.orig.basic test:
- DROP EXTENSION IF EXISTS mage; when not installed -> NOTICE skip
- DROP EXTENSION mage; when not installed -> proper
"extension does not exist" error, not the ag_graph one
- CREATE EXTENSION mage; then DROP EXTENSION IF EXISTS mage;
confirms the AGE cleanup path still runs when installed
* java/yb-pgsql/src/test/java/org/yb/pgsql/TestYbBackup.java:
testMageBackupRestore does an end-to-end YBC backup/restore round
trip on a database with mage installed
Adds a class-level ysql_yb_enable_mage=true tserver flag; harmless
to the other tests in the class, which never CREATE EXTENSION mage.
Verified locally on a 3-node mini-cluster:
* mage regress (TestPgRegressThirdPartyExtensionsMage): 1/1 pass.
* TestYbBackup#testMageBackupRestore under YB_TEST_YB_CONTROLLER=1:
|
trigger jenkins |
|
Jenkins build has been triggered. Results will be available in the CI checks. CSI |
karthik-ramanathan-3006
left a comment
karthik-ramanathan-3006
left a comment
There was a problem hiding this comment.
Overall LGTM.
Wouldn't this also be a problem for upstream Apache AGE?
Would it be worth contributing an upstream fix? Or have they already fixed it?
Co-authored-by: Karthik Ramanathan <32414766+karthik-ramanathan-3006@users.noreply.github.com>
|
trigger jenkins |
|
Jenkins build has been triggered. Results will be available in the CI checks. CSI |
2 participants
DO NOT MERGE!! CSI ⏳
sql_dump in binary-upgrade mode emits
before recreating an empty extension shell, so any backup/restore of a database that uses the mage (Apache AGE) extension hits this statement during restore. The mage ProcessUtility hook intercepts every DROP that mentions "mage" via is_age_drop() and unconditionally routes it through drop_age_extension(), which enumerates graphs from mag_catalog.ag_graph. When the extension is not (yet) installed on the restore target, that lookup errors out with
and the whole restore aborts, so a backup of a mage-using database was effectively unrestorable.
Fix: guard is_age_drop() with get_extension_oid("mage", true). When the extension is not installed, defer to standard ProcessUtility, which correctly performs the IF EXISTS no-op (or raises the standard "extension does not exist" error for a bare DROP EXTENSION mage). get_extension_oid is preferred over checking the mag_catalog schema: the schema can linger after a prior DROP EXTENSION (so a schema-based guard still ran the AGE drop path and still hit the ag_graph lookup).
Tests:
src/postgres/third-party-extensions/mage/regress/{sql,expected}/ yb.orig.basic.{sql,out}: three direct regress cases appended at the end of the existing yb.orig.basic test: - DROP EXTENSION IF EXISTS mage; when not installed -> NOTICE skip
"extension does not exist" error, not the ag_graph one
confirms the AGE cleanup path still runs when installed
java/yb-pgsql/src/test/java/org/yb/pgsql/TestYbBackup.java: testMageBackupRestore does an end-to-end YBC backup/restore round trip on a database with mage installed Adds a class-level ysql_yb_enable_mage=true tserver flag; harmless to the other tests in the class, which never CREATE EXTENSION mage.
Verified locally on a 3-node mini-cluster: