Skip to content

Commit 340a0dd

Browse files
committed
Make age extension usable from shared_preload_libraries
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.
1 parent 23cbe57 commit 340a0dd

7 files changed

Lines changed: 202 additions & 60 deletions

File tree

regress/expected/drop.out

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,14 @@ SELECT tablename FROM pg_catalog.pg_tables WHERE schemaname = 'ag_catalog';
4040
-----------
4141
(0 rows)
4242

43+
-- When ag_catalog is missing extension hooks shouldn't fail with the
44+
-- ERROR schema "ag_catalog" does not exist.
45+
-- It might happen when 'age' is loaded but extension isn't created yet.
46+
SET client_min_messages TO WARNING;
47+
DROP SCHEMA IF EXISTS ag_catalog CASCADE;
48+
RESET client_min_messages;
49+
CREATE SCHEMA _regress_drop;
50+
DROP SCHEMA _regress_drop; -- should'n produce the ERROR
4351
-- Recreate the extension and validate we can recreate a graph
4452
CREATE EXTENSION age;
4553
SELECT create_graph('drop');
@@ -115,7 +123,7 @@ NOTICE: label "issue_1305"."r" has been dropped
115123
(1 row)
116124

117125
SELECT drop_label('issue_1305', 'r');
118-
ERROR: rel_name not found for label "r"
126+
ERROR: label "r" does not exist
119127
SELECT drop_label('issue_1305', 'n', false);
120128
NOTICE: label "issue_1305"."n" has been dropped
121129
drop_label
@@ -124,7 +132,7 @@ NOTICE: label "issue_1305"."n" has been dropped
124132
(1 row)
125133

126134
SELECT drop_label('issue_1305', 'n');
127-
ERROR: rel_name not found for label "n"
135+
ERROR: label "n" does not exist
128136
SELECT * FROM drop_graph('issue_1305', true);
129137
NOTICE: drop cascades to 2 other objects
130138
DETAIL: drop cascades to table issue_1305._ag_label_vertex

regress/sql/drop.sql

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,15 @@ SELECT nspname FROM pg_catalog.pg_namespace WHERE nspname = 'drop';
2828

2929
SELECT tablename FROM pg_catalog.pg_tables WHERE schemaname = 'ag_catalog';
3030

31+
-- When ag_catalog is missing extension hooks shouldn't fail with the
32+
-- ERROR schema "ag_catalog" does not exist.
33+
-- It might happen when 'age' is loaded but extension isn't created yet.
34+
SET client_min_messages TO WARNING;
35+
DROP SCHEMA IF EXISTS ag_catalog CASCADE;
36+
RESET client_min_messages;
37+
CREATE SCHEMA _regress_drop;
38+
DROP SCHEMA _regress_drop; -- should'n produce the ERROR
39+
3140
-- Recreate the extension and validate we can recreate a graph
3241
CREATE EXTENSION age;
3342

src/backend/age.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
* under the License.
1818
*/
1919

20+
#include "postgres.h"
21+
#include "miscadmin.h"
22+
2023
#include "catalog/ag_catalog.h"
2124
#include "nodes/ag_nodes.h"
2225
#include "optimizer/cypher_paths.h"
@@ -25,7 +28,6 @@
2528
#include "utils/age_global_graph.h"
2629

2730
#if PG_VERSION_NUM < 170000
28-
#include "miscadmin.h"
2931

3032
/* saved hook pointers for PG < 17 shmem path */
3133
static shmem_request_hook_type prev_shmem_request_hook = NULL;
@@ -56,6 +58,11 @@ void _PG_init(void);
5658

5759
void _PG_init(void)
5860
{
61+
if (IsBinaryUpgrade)
62+
{
63+
return;
64+
}
65+
5966
register_ag_nodes();
6067
set_rel_pathlist_init();
6168
object_access_hook_init();

src/backend/catalog/ag_catalog.c

Lines changed: 159 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -19,21 +19,27 @@
1919

2020
#include "postgres.h"
2121

22+
#include "access/xact.h"
2223
#include "catalog/dependency.h"
2324
#include "catalog/namespace.h"
2425
#include "catalog/objectaccess.h"
2526
#include "catalog/pg_class_d.h"
27+
#include "catalog/pg_extension_d.h"
2628
#include "catalog/pg_namespace_d.h"
2729
#include "commands/defrem.h"
30+
#include "commands/extension.h"
2831
#include "nodes/parsenodes.h"
2932
#include "tcop/utility.h"
33+
#include "utils/inval.h"
3034
#include "utils/lsyscache.h"
3135

3236
#include "catalog/ag_graph.h"
3337
#include "catalog/ag_label.h"
3438
#include "utils/ag_cache.h"
3539
#include "utils/age_global_graph.h"
3640

41+
static bool extension_cache_is_valid = false;
42+
static bool age_extension_exists = false;
3743
static object_access_hook_type prev_object_access_hook;
3844
static ProcessUtility_hook_type prev_process_utility_hook;
3945
static bool prev_object_hook_is_set;
@@ -45,8 +51,46 @@ void ag_ProcessUtility_hook(PlannedStmt *pstmt, const char *queryString, bool re
4551
QueryEnvironment *queryEnv, DestReceiver *dest,
4652
QueryCompletion *qc);
4753

48-
static bool is_age_drop(PlannedStmt *pstmt);
49-
static void drop_age_extension(DropStmt *stmt);
54+
static bool is_age_drop(DropStmt *drop_stmt);
55+
56+
static void
57+
invalidate_extension_cache_callback(Datum argument, Oid relationId)
58+
{
59+
if (!OidIsValid(relationId) || relationId == ExtensionRelationId)
60+
{
61+
extension_cache_is_valid = false;
62+
}
63+
}
64+
65+
/*
66+
* We don't want most of hooks to do anything if the "age" extension isn't
67+
* created. However, scanning pg_extension is a costly operation, therefore we
68+
* implement a caching mechanism and reset it with the help of the relcache
69+
* callback mechanism.
70+
*
71+
* Please also see ag_ProcessUtility_hook() function for more details.
72+
*/
73+
bool
74+
is_age_extension_exists(void)
75+
{
76+
static bool callback_registered = false;
77+
78+
if (extension_cache_is_valid)
79+
return age_extension_exists;
80+
81+
if (!callback_registered)
82+
{
83+
CacheRegisterRelcacheCallback(invalidate_extension_cache_callback,
84+
(Datum) 0);
85+
callback_registered = true;
86+
}
87+
88+
age_extension_exists = OidIsValid(get_extension_oid("age", true));
89+
90+
extension_cache_is_valid = true;
91+
92+
return age_extension_exists;
93+
}
5094

5195
void object_access_hook_init(void)
5296
{
@@ -86,50 +130,97 @@ void process_utility_hook_fini(void)
86130
* information in the indexes and tables being dropped. To prevent an error
87131
* from being thrown, we need to disable the object_access_hook before dropping
88132
* the extension.
133+
*
134+
* Besides that, we want to notify other backends about the fact that "age"
135+
* extension was probably created/dropped so that they can enable/disable
136+
* hooks.
89137
*/
90138
void ag_ProcessUtility_hook(PlannedStmt *pstmt, const char *queryString,
91139
bool readOnlyTree, ProcessUtilityContext context,
92140
ParamListInfo params, QueryEnvironment *queryEnv,
93141
DestReceiver *dest, QueryCompletion *qc)
94142
{
95-
if (is_age_drop(pstmt))
96-
{
97-
drop_age_extension((DropStmt *)pstmt->utilityStmt);
98-
}
99-
else
143+
bool creating_age = false;
144+
bool dropping_age = false;
145+
146+
if (!IsAbortedTransactionBlockState())
100147
{
101-
/*
102-
* Check for TRUNCATE on graph label tables. If any truncated
103-
* table is a graph label table, increment the version counter
104-
* for that graph to invalidate VLE caches. We do this before
105-
* the truncate executes so the cache is invalidated regardless.
106-
*/
107-
if (IsA(pstmt->utilityStmt, TruncateStmt))
148+
Node *parsetree = pstmt->utilityStmt;
149+
150+
switch (nodeTag(parsetree))
108151
{
109-
TruncateStmt *tstmt = (TruncateStmt *) pstmt->utilityStmt;
110-
ListCell *lc;
152+
case T_CreateExtensionStmt:
153+
{
154+
CreateExtensionStmt *stmt =
155+
(CreateExtensionStmt *) parsetree;
156+
creating_age = strcmp(stmt->extname, "age") == 0;
157+
}
158+
break;
159+
case T_DropStmt:
160+
{
161+
DropStmt *stmt = (DropStmt *) parsetree;
111162

112-
foreach(lc, tstmt->relations)
113-
{
114-
RangeVar *rv = (RangeVar *) lfirst(lc);
115-
Oid rel_oid = RangeVarGetRelid(rv, AccessShareLock, true);
163+
if (stmt->removeType != OBJECT_EXTENSION)
164+
break;
116165

117-
if (OidIsValid(rel_oid))
118-
{
119-
Oid graph_oid = get_graph_oid_for_table(rel_oid);
166+
if (!is_age_drop(stmt))
167+
break;
120168

121-
if (OidIsValid(graph_oid))
169+
dropping_age = true;
170+
}
171+
break;
172+
case T_TruncateStmt:
173+
{
174+
/*
175+
* Check for TRUNCATE on graph label tables. If any
176+
* truncated table is a graph label table, increment the
177+
* version counter for that graph to invalidate VLE caches.
178+
* We do this before the truncate executes so the cache is
179+
* invalidated regardless.
180+
*/
181+
TruncateStmt *tstmt = (TruncateStmt *) parsetree;
182+
ListCell *lc;
183+
184+
foreach(lc, tstmt->relations)
122185
{
123-
increment_graph_version(graph_oid);
186+
RangeVar *rv = (RangeVar *) lfirst(lc);
187+
Oid rel_oid = RangeVarGetRelid(rv, AccessShareLock,
188+
true);
189+
190+
if (OidIsValid(rel_oid))
191+
{
192+
Oid graph_oid =
193+
get_graph_oid_for_table(rel_oid);
194+
195+
if (OidIsValid(graph_oid))
196+
{
197+
increment_graph_version(graph_oid);
198+
}
199+
}
124200
}
125201
}
126-
}
202+
break;
203+
default:
204+
break;
127205
}
206+
}
207+
208+
if (dropping_age)
209+
{
210+
/* Remove all graphs */
211+
drop_graphs(get_graphnames());
128212

213+
/* Remove the object access hook */
214+
object_access_hook_fini();
215+
}
216+
217+
PG_TRY();
218+
{
129219
if (prev_process_utility_hook)
130220
{
131221
(*prev_process_utility_hook) (pstmt, queryString, readOnlyTree,
132-
context, params, queryEnv, dest, qc);
222+
context, params, queryEnv, dest,
223+
qc);
133224
}
134225
else
135226
{
@@ -141,38 +232,47 @@ void ag_ProcessUtility_hook(PlannedStmt *pstmt, const char *queryString,
141232
params, queryEnv, dest, qc);
142233
}
143234
}
144-
}
145-
146-
static void drop_age_extension(DropStmt *stmt)
147-
{
148-
/* Remove all graphs */
149-
drop_graphs(get_graphnames());
235+
PG_CATCH();
236+
{
237+
if (dropping_age)
238+
{
239+
/*
240+
* We have to restore the disabled object_access_hook if
241+
* DROP EXTENSION age failed.
242+
*/
243+
object_access_hook_init();
244+
}
245+
PG_RE_THROW();
246+
}
247+
PG_END_TRY();
150248

151-
/* Remove the object access hook */
152-
object_access_hook_fini();
249+
if (dropping_age)
250+
{
251+
/* reset global variables for OIDs */
252+
clear_global_Oids_AGTYPE();
253+
clear_global_Oids_GRAPHID();
254+
clear_global_Oids_VERTEX_EDGE();
153255

154-
/*
155-
* Run Postgres' logic to perform the remaining work to drop the
156-
* extension.
157-
*/
158-
RemoveObjects(stmt);
256+
/* Restore the object access hook */
257+
object_access_hook_init();
258+
}
159259

160-
/* reset global variables for OIDs */
161-
clear_global_Oids_AGTYPE();
162-
clear_global_Oids_GRAPHID();
163-
clear_global_Oids_VERTEX_EDGE();
260+
if (creating_age || dropping_age)
261+
{
262+
/* Notify all backends that pg_extension was modified. */
263+
CacheInvalidateRelcacheByRelid(ExtensionRelationId);
264+
}
164265
}
165266

166267
/* Check to see if the Utility Command is to drop the AGE Extension. */
167-
static bool is_age_drop(PlannedStmt *pstmt)
268+
static bool is_age_drop(DropStmt *drop_stmt)
168269
{
169270
ListCell *lc;
170-
DropStmt *drop_stmt;
171271

172-
if (!IsA(pstmt->utilityStmt, DropStmt))
272+
if (!is_age_extension_exists())
273+
{
173274
return false;
174-
175-
drop_stmt = (DropStmt *)pstmt->utilityStmt;
275+
}
176276

177277
foreach(lc, drop_stmt->objects)
178278
{
@@ -183,8 +283,10 @@ static bool is_age_drop(PlannedStmt *pstmt)
183283
String *val = (String *)obj;
184284
char *str = val->sval;
185285

186-
if (!pg_strcasecmp(str, "age"))
286+
if (strcmp(str, "age") == 0)
287+
{
187288
return true;
289+
}
188290
}
189291
}
190292

@@ -205,16 +307,16 @@ static void object_access(ObjectAccessType access, Oid class_id, Oid object_id,
205307
if (prev_object_access_hook)
206308
prev_object_access_hook(access, class_id, object_id, sub_id, arg);
207309

208-
/* We are interested in DROP SCHEMA and DROP TABLE commands. */
209-
if (access != OAT_DROP)
310+
if (!is_age_extension_exists())
311+
{
210312
return;
313+
}
211314

212-
/*
213-
* Age might be installed into shared_preload_libraries before extension is
214-
* created. In this case we must bail out from this hook.
215-
*/
216-
if (!OidIsValid(get_namespace_oid("ag_catalog", true)))
315+
/* We are interested in DROP SCHEMA and DROP TABLE commands. */
316+
if (access != OAT_DROP)
317+
{
217318
return;
319+
}
218320

219321
drop_arg = arg;
220322

0 commit comments

Comments
 (0)