Copilot commented on code in PR #2438:
URL: https://github.com/apache/age/pull/2438#discussion_r3430706515
##########
src/backend/catalog/ag_catalog.c:
##########
@@ -45,8 +51,44 @@ void ag_ProcessUtility_hook(PlannedStmt *pstmt, const char
*queryString, bool re
QueryEnvironment *queryEnv, DestReceiver *dest,
QueryCompletion *qc);
-static bool is_age_drop(PlannedStmt *pstmt);
-static void drop_age_extension(DropStmt *stmt);
+static bool is_age_drop(DropStmt *drop_stmt);
+
+static void
+invalidate_extension_cache_callback(Datum argument, Oid relationId)
+{
+ if (relationId == ExtensionRelationId)
+ extension_cache_is_valid = false;
+}
Review Comment:
The relcache callback can be invoked with InvalidOid to indicate a global
relcache invalidation (see invalidate_label_caches in ag_cache.c). If that
happens, the extension-existence cache would remain marked valid and could
become stale.
##########
src/include/catalog/ag_catalog.h:
##########
@@ -24,6 +24,8 @@
#include "utils/agtype.h"
+bool is_age_extension_exist(void);
Review Comment:
Function name `is_age_extension_exist` is inconsistent with the codebase’s
common `*_exists` naming (e.g., graph_exists, label_id_exists, entity_exists).
Consider renaming to `is_age_extension_exists` or similar for consistency,
updating the header and all call sites.
##########
regress/expected/drop.out:
##########
@@ -40,6 +40,12 @@ SELECT tablename FROM pg_catalog.pg_tables WHERE schemaname
= 'ag_catalog';
-----------
(0 rows)
+-- 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;
Review Comment:
Expected output should match the SQL change to `DROP SCHEMA IF EXISTS
ag_catalog` to avoid brittle failures if ag_catalog is already missing.
##########
regress/sql/drop.sql:
##########
@@ -28,6 +28,13 @@ SELECT nspname FROM pg_catalog.pg_namespace WHERE nspname =
'drop';
SELECT tablename FROM pg_catalog.pg_tables WHERE schemaname = 'ag_catalog';
+-- 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;
Review Comment:
To keep the regression test resilient (e.g., if ag_catalog is already absent
after DROP EXTENSION behavior changes), use `DROP SCHEMA IF EXISTS` here. The
test still achieves the goal of ensuring ag_catalog is missing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
