Make age extension usable from shared_preload_libraries#2438
Make age extension usable from shared_preload_libraries#2438serdarmumcu wants to merge 1 commit into
Conversation
|
@serdarmumcu The build has an error -
This needs to be fixed. |
There was a problem hiding this comment.
Pull request overview
This PR makes the AGE shared library safe to load via shared_preload_libraries by ensuring its hooks become no-ops until the age extension is actually installed, avoiding failures when ag_catalog doesn’t exist yet.
Changes:
- Adds a cached
pg_extensionexistence check and uses it to guard key hooks (post_parse_analyze,set_rel_pathlist,object_access). - Updates
ProcessUtilityhook logic to detectCREATE/DROP EXTENSION age, invalidate the cached state across backends, and safely restore hooks on failure. - Extends regression coverage to validate hook behavior when
ag_catalogis absent and improves cleanup in VLE regression SQL.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/include/catalog/ag_catalog.h | Exposes the new AGE-extension existence check API. |
| src/backend/parser/cypher_analyze.c | Guards post-parse analysis hook when AGE extension isn’t installed. |
| src/backend/optimizer/cypher_paths.c | Guards planner hook when AGE extension isn’t installed. |
| src/backend/catalog/ag_catalog.c | Implements extension-existence caching, hook guarding, and cross-backend invalidation on create/drop. |
| src/backend/age.c | Skips _PG_init during binary upgrade to avoid hook registration during pg_upgrade. |
| regress/sql/drop.sql | Adds regression coverage for hook safety when ag_catalog is missing. |
| regress/sql/cypher_vle.sql | Adds missing cleanup for helper functions created during the test. |
| regress/expected/drop.out / regress/expected/cypher_vle.out | Updates expected outputs to match the new regression behavior. |
Comments suppressed due to low confidence (1)
src/backend/catalog/ag_catalog.c:272
- drop_age_extension() is now unused (no remaining call sites). With the project building with COPT=-Werror in CI, an unused static function is likely to trigger -Wunused-function and fail the build; the forward declaration at the top should be removed as well.
static void drop_age_extension(DropStmt *stmt)
{
/* Remove all graphs */
drop_graphs(get_graphnames());
/* Remove the object access hook */
object_access_hook_fini();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| static void | ||
| invalidate_extension_cache_callback(Datum argument, Oid relationId) | ||
| { | ||
| if (relationId == ExtensionRelationId) | ||
| extension_cache_is_valid = false; | ||
| } |
|
|
||
| #include "utils/agtype.h" | ||
|
|
||
| bool is_age_extension_exist(void); |
| -- When ag_catalog is missing extension hooks shouldn't fail with the | ||
| -- ERROR schema "ag_catalog" does not exist. | ||
| -- It might happen when 'age' is loaded but extension isn't created yet. | ||
| DROP SCHEMA ag_catalog; |
When AGE is loaded via shared_preload_libraries, its hooks (post_parse_analyze, set_rel_pathlist, object_access) are active before CREATE EXTENSION age is run. This causes errors when non-Cypher queries trigger those hooks and ag_catalog does not yet exist. Changes: - Add is_age_extension_exist() with a relcache callback cache so that checking pg_extension is not repeated on every hook invocation. - Guard post_parse_analyze, set_rel_pathlist, and object_access hooks with is_age_extension_exist() so they become no-ops when the extension is not installed. - Refactor ag_ProcessUtility_hook to detect CREATE/DROP EXTENSION age and broadcast a relcache invalidation via CacheInvalidateRelcacheByRelid(ExtensionRelationId) so other backends update their cached extension state. - Wrap DROP EXTENSION processing in PG_TRY/PG_CATCH to restore object_access_hook if the drop fails (e.g. dependent objects). - Skip _PG_init during pg_upgrade (IsBinaryUpgrade) to avoid hook registration when the binary-upgrade machinery is running. - Add regression tests that verify hooks do not error when ag_catalog schema is absent.
1bf9ff5 to
bc56b00
Compare
@jrgemignani Fixed — removed the dead drop_age_extension() function and also cleaned up unrelated changes from internal branches that had leaked into the PR. The commit now only contains changes related to shared_preload_libraries support. Ready for re-review. |


When AGE is loaded via
shared_preload_libraries, its hooks (post_parse_analyze,set_rel_pathlist,object_access) are active beforeCREATE EXTENSION ageis run. This causes errors when non-Cypher queries trigger those hooks andag_catalogdoes not yet exist.Changes
is_age_extension_exist()with a relcache callback cache so that checkingpg_extensionis not repeated on every hook invocation.post_parse_analyze,set_rel_pathlist, andobject_accesshooks withis_age_extension_exist()so they become no-ops when the extension is not installed.ag_ProcessUtility_hookto detectCREATE/DROP EXTENSION ageand broadcast a relcache invalidation viaCacheInvalidateRelcacheByRelid(ExtensionRelationId)so other backends update their cached extension state.DROP EXTENSIONprocessing inPG_TRY/PG_CATCHto restoreobject_access_hookif the drop fails (e.g. dependent objects)._PG_initduringpg_upgrade(IsBinaryUpgrade) to avoid hook registration when the binary-upgrade machinery is running.ag_catalogschema is absent.