public inbox for [email protected]
help / color / mirror / Atom feedAllow a condition string in an injection point
3+ messages / 2 participants
[nested] [flat]
* Allow a condition string in an injection point
@ 2026-04-09 18:02 Sami Imseih <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Sami Imseih @ 2026-04-09 18:02 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Michael Paquier <[email protected]>; Masahiko Sawada <[email protected]>; Daniil Davydov <[email protected]>
A follow-up to the discussion here [0], here is a patch that allows
for an arbitrary string in injection points to be able to apply more
granular filters for running an injection point. This will be useful
for autovacuum testing as discussed in the referenced thread,
and perhaps in some other places.
The string is capped at 256 bytes which seems like a reasonable
value. I considered using a flexible_array_member and to track
the length, but that hardly seemed worth it at this stage.
A case I envision, and there could be more is only run
the injection point for a specific rel.
```
SELECT injection_points_attach('my-inj-pt', 'wait', 'tab1');
```
```
#ifdef USE_INJECTION_POINTS
INJECTION_POINT("my-inj-pt", RelationGetRelationName(rel));
#endif
```
Worth noting, the condition types were changed to bit flags since
we may need to combine conditions such as local injection point
and string.
typedef enum InjectionPointConditionType
{
- INJ_CONDITION_ALWAYS = 0, /* always run */
- INJ_CONDITION_PID, /* PID restriction */
+ INJ_CONDITION_PID = 1 << 0, /* PID restriction */
+ INJ_CONDITION_STRING = 1 << 1, /* generic string match against arg */
} InjectionPointConditionType;
--
Sami
[0] https://www.postgresql.org/message-id/adWcVrX3jrHPoCmD%40paquier.xyz
Attachments:
[application/octet-stream] v1-0001-Allow-a-condition-string-in-an-injection-point.patch (11.2K, 2-v1-0001-Allow-a-condition-string-in-an-injection-point.patch)
download | inline diff:
From 33149505dab4e7b8f7025e9e37711b9bfb33c7ad Mon Sep 17 00:00:00 2001
From: Sami Imseih <[email protected]>
Date: Thu, 9 Apr 2026 17:19:45 +0000
Subject: [PATCH v1 1/1] Allow a condition string in an injection point
This allows a caller of InjectionPointAttach to pass
an arbitrary string which can be later matched when the
injection point is run. This allows for more flexible
conditions to be used for running an injection point.
For example, this could be used to match by a relname
or OID. The condition string is capped at 256 bytes
which seems good enough, but this could be bumped up
in the future. Using a variable-length string may be
useful, but seemed hardly worth it at this point.
---
src/backend/utils/misc/injection_point.c | 1 +
.../expected/injection_points.out | 68 +++++++++++++++++++
.../injection_points--1.0.sql | 5 +-
.../injection_points/injection_points.c | 55 +++++++++------
.../injection_points/sql/injection_points.sql | 17 +++++
5 files changed, 124 insertions(+), 22 deletions(-)
diff --git a/src/backend/utils/misc/injection_point.c b/src/backend/utils/misc/injection_point.c
index 9d30843a111..272ef5e578a 100644
--- a/src/backend/utils/misc/injection_point.c
+++ b/src/backend/utils/misc/injection_point.c
@@ -322,6 +322,7 @@ InjectionPointAttach(const char *name,
strlcpy(entry->name, name, sizeof(entry->name));
strlcpy(entry->library, library, sizeof(entry->library));
strlcpy(entry->function, function, sizeof(entry->function));
+ memset(entry->private_data, 0, INJ_PRIVATE_MAXLEN);
if (private_data != NULL)
memcpy(entry->private_data, private_data, private_data_size);
diff --git a/src/test/modules/injection_points/expected/injection_points.out b/src/test/modules/injection_points/expected/injection_points.out
index a3ccaee5472..0f5c1726d3a 100644
--- a/src/test/modules/injection_points/expected/injection_points.out
+++ b/src/test/modules/injection_points/expected/injection_points.out
@@ -262,6 +262,32 @@ NOTICE: notice triggered for injection point TestConditionLocal2
(1 row)
+-- Local injection point condition string matching.
+SELECT injection_points_attach('TestConditionLocalString', 'notice', 'LocalData');
+ injection_points_attach
+-------------------------
+
+(1 row)
+
+SELECT injection_points_run('TestConditionLocalString', 'LocalData'); -- notice
+NOTICE: notice triggered for injection point TestConditionLocalString (LocalData)
+ injection_points_run
+----------------------
+
+(1 row)
+
+SELECT injection_points_run('TestConditionLocalString', 'WrongData'); -- nothing
+ injection_points_run
+----------------------
+
+(1 row)
+
+SELECT injection_points_run('TestConditionLocalString', NULL); -- nothing
+ injection_points_run
+----------------------
+
+(1 row)
+
SELECT pg_backend_pid() AS oldpid \gset
-- reload, local injection points should be gone.
\c
@@ -285,6 +311,12 @@ SELECT injection_points_run('TestConditionLocal2'); -- nothing
(1 row)
+SELECT injection_points_run('TestConditionLocalString', 'LocalData'); -- nothing
+ injection_points_run
+----------------------
+
+(1 row)
+
SELECT injection_points_run('TestConditionError'); -- error
ERROR: error triggered for injection point TestConditionError
SELECT injection_points_detach('TestConditionError');
@@ -307,6 +339,42 @@ SELECT injection_points_detach('TestConditionLocal1');
(1 row)
+-- injection point condition string matching.
+SELECT injection_points_attach('TestConditionString', 'notice', 'MyString');
+ injection_points_attach
+-------------------------
+
+(1 row)
+
+SELECT injection_points_run('TestConditionString', 'MyString'); -- notice
+NOTICE: notice triggered for injection point TestConditionString (MyString)
+ injection_points_run
+----------------------
+
+(1 row)
+
+SELECT injection_points_run('TestConditionString', 'WrongString'); -- nothing
+ injection_points_run
+----------------------
+
+(1 row)
+
+SELECT injection_points_run('TestConditionString', NULL); -- nothing
+ injection_points_run
+----------------------
+
+(1 row)
+
+SELECT injection_points_detach('TestConditionString');
+ injection_points_detach
+-------------------------
+
+(1 row)
+
+-- condition string too long for attach.
+SELECT injection_points_attach('TestInjectionError', 'error', repeat('a', 256));
+ERROR: injection point condition string too long
+DETAIL: injection point condition string must be less than 256 characters.
-- Function variant for attach.
SELECT injection_points_attach(repeat('a', 64), 'injection_points',
'injection_notice', NULL);
diff --git a/src/test/modules/injection_points/injection_points--1.0.sql b/src/test/modules/injection_points/injection_points--1.0.sql
index 861c7355d4e..2efb307f5bf 100644
--- a/src/test/modules/injection_points/injection_points--1.0.sql
+++ b/src/test/modules/injection_points/injection_points--1.0.sql
@@ -9,10 +9,11 @@
-- Attaches the action to the given injection point.
--
CREATE FUNCTION injection_points_attach(IN point_name TEXT,
- IN action text)
+ IN action text,
+ IN condition_string text DEFAULT NULL)
RETURNS void
AS 'MODULE_PATHNAME', 'injection_points_attach'
-LANGUAGE C STRICT PARALLEL UNSAFE;
+LANGUAGE C PARALLEL UNSAFE;
--
-- injection_points_attach()
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index 0f1af513673..22b902a4729 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -39,6 +39,7 @@ PG_MODULE_MAGIC;
/* Maximum number of waits usable in injection points at once */
#define INJ_MAX_WAIT 8
#define INJ_NAME_MAXLEN 64
+#define INJ_DATA_MAXLEN 256
/*
* Conditions related to injection points. This tracks in shared memory the
@@ -51,8 +52,8 @@ PG_MODULE_MAGIC;
*/
typedef enum InjectionPointConditionType
{
- INJ_CONDITION_ALWAYS = 0, /* always run */
- INJ_CONDITION_PID, /* PID restriction */
+ INJ_CONDITION_PID = 1 << 0, /* PID restriction */
+ INJ_CONDITION_STRING = 1 << 1, /* generic string match against arg */
} InjectionPointConditionType;
typedef struct InjectionPointCondition
@@ -62,6 +63,9 @@ typedef struct InjectionPointCondition
/* ID of the process where the injection point is allowed to run */
int pid;
+
+ /* String to match against the argument at run time */
+ char str[INJ_DATA_MAXLEN];
} InjectionPointCondition;
/*
@@ -173,21 +177,21 @@ injection_init_shmem(void)
* otherwise.
*/
static bool
-injection_point_allowed(const InjectionPointCondition *condition)
+injection_point_allowed(const InjectionPointCondition *condition,
+ const char *arg)
{
- bool result = true;
-
- switch (condition->type)
- {
- case INJ_CONDITION_PID:
- if (MyProcPid != condition->pid)
- result = false;
- break;
- case INJ_CONDITION_ALWAYS:
- break;
- }
-
- return result;
+ /* did not match the condition PID */
+ if ((condition->type & INJ_CONDITION_PID) &&
+ MyProcPid != condition->pid)
+ return false;
+
+ /* did not match the condition string */
+ if ((condition->type & INJ_CONDITION_STRING) &&
+ (condition->str[0] == '\0' ||
+ arg == NULL || strcmp(condition->str, arg) != 0))
+ return false;
+
+ return true;
}
/*
@@ -219,7 +223,7 @@ injection_error(const char *name, const void *private_data, void *arg)
const InjectionPointCondition *condition = private_data;
char *argstr = arg;
- if (!injection_point_allowed(condition))
+ if (!injection_point_allowed(condition, argstr))
return;
if (argstr)
@@ -235,7 +239,7 @@ injection_notice(const char *name, const void *private_data, void *arg)
const InjectionPointCondition *condition = private_data;
char *argstr = arg;
- if (!injection_point_allowed(condition))
+ if (!injection_point_allowed(condition, argstr))
return;
if (argstr)
@@ -257,7 +261,7 @@ injection_wait(const char *name, const void *private_data, void *arg)
if (inj_state == NULL)
injection_init_shmem();
- if (!injection_point_allowed(condition))
+ if (!injection_point_allowed(condition, arg))
return;
/*
@@ -318,6 +322,7 @@ injection_points_attach(PG_FUNCTION_ARGS)
{
char *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
char *action = text_to_cstring(PG_GETARG_TEXT_PP(1));
+ char *str = PG_ARGISNULL(2) ? NULL : text_to_cstring(PG_GETARG_TEXT_PP(2));
char *function;
InjectionPointCondition condition = {0};
@@ -332,10 +337,20 @@ injection_points_attach(PG_FUNCTION_ARGS)
if (injection_point_local)
{
- condition.type = INJ_CONDITION_PID;
+ condition.type |= INJ_CONDITION_PID;
condition.pid = MyProcPid;
}
+ if (str)
+ {
+ if (strlen(str) >= INJ_DATA_MAXLEN)
+ ereport(ERROR,
+ (errmsg("injection point condition string too long"),
+ errdetail("injection point condition string must be less than %d characters.", INJ_DATA_MAXLEN)));
+ condition.type |= INJ_CONDITION_STRING;
+ strlcpy(condition.str, str, INJ_DATA_MAXLEN);
+ }
+
InjectionPointAttach(name, "injection_points", function, &condition,
sizeof(InjectionPointCondition));
diff --git a/src/test/modules/injection_points/sql/injection_points.sql b/src/test/modules/injection_points/sql/injection_points.sql
index ba14df706ef..805936f7064 100644
--- a/src/test/modules/injection_points/sql/injection_points.sql
+++ b/src/test/modules/injection_points/sql/injection_points.sql
@@ -72,6 +72,12 @@ SELECT injection_points_attach('TestConditionLocal2', 'notice');
SELECT injection_points_run('TestConditionLocal1'); -- error
SELECT injection_points_run('TestConditionLocal2'); -- notice
+-- Local injection point condition string matching.
+SELECT injection_points_attach('TestConditionLocalString', 'notice', 'LocalData');
+SELECT injection_points_run('TestConditionLocalString', 'LocalData'); -- notice
+SELECT injection_points_run('TestConditionLocalString', 'WrongData'); -- nothing
+SELECT injection_points_run('TestConditionLocalString', NULL); -- nothing
+
SELECT pg_backend_pid() AS oldpid \gset
-- reload, local injection points should be gone.
@@ -81,6 +87,7 @@ SELECT pg_backend_pid() AS oldpid \gset
SELECT wait_pid(:'oldpid');
SELECT injection_points_run('TestConditionLocal1'); -- nothing
SELECT injection_points_run('TestConditionLocal2'); -- nothing
+SELECT injection_points_run('TestConditionLocalString', 'LocalData'); -- nothing
SELECT injection_points_run('TestConditionError'); -- error
SELECT injection_points_detach('TestConditionError');
-- Attaching injection points that use the same name as one defined locally
@@ -88,6 +95,16 @@ SELECT injection_points_detach('TestConditionError');
SELECT injection_points_attach('TestConditionLocal1', 'error');
SELECT injection_points_detach('TestConditionLocal1');
+-- injection point condition string matching.
+SELECT injection_points_attach('TestConditionString', 'notice', 'MyString');
+SELECT injection_points_run('TestConditionString', 'MyString'); -- notice
+SELECT injection_points_run('TestConditionString', 'WrongString'); -- nothing
+SELECT injection_points_run('TestConditionString', NULL); -- nothing
+SELECT injection_points_detach('TestConditionString');
+
+-- condition string too long for attach.
+SELECT injection_points_attach('TestInjectionError', 'error', repeat('a', 256));
+
-- Function variant for attach.
SELECT injection_points_attach(repeat('a', 64), 'injection_points',
'injection_notice', NULL);
--
2.50.1
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Allow a condition string in an injection point
@ 2026-04-09 22:38 Michael Paquier <[email protected]>
parent: Sami Imseih <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Michael Paquier @ 2026-04-09 22:38 UTC (permalink / raw)
To: Sami Imseih <[email protected]>; +Cc: pgsql-hackers; Masahiko Sawada <[email protected]>; Daniil Davydov <[email protected]>
On Thu, Apr 09, 2026 at 01:02:37PM -0500, Sami Imseih wrote:
> A follow-up to the discussion here [0], here is a patch that allows
> for an arbitrary string in injection points to be able to apply more
> granular filters for running an injection point. This will be useful
> for autovacuum testing as discussed in the referenced thread,
> and perhaps in some other places.
Are the patches under discussion required for v19 or is that something
that can wait before v20 opens for business? We have always required
a use-case in core before adding a new API in this module, to justify
its existence.
> The string is capped at 256 bytes which seems like a reasonable
> value. I considered using a flexible_array_member and to track
> the length, but that hardly seemed worth it at this stage.
This change is local to the module injection_points. That can be
changed at will even in the back-branches.
> A case I envision, and there could be more is only run
> the injection point for a specific rel.
>
> SELECT injection_points_attach('my-inj-pt', 'wait', 'tab1');
We may be able to simplify a couple more places that rely on multiple
INJECTION_POINT currently, with slightly different names. I doubt
these currently in the tree are worth changing. That would make
backpatches more invasive for the tests, which is more noise, so
most likely no.
> #ifdef USE_INJECTION_POINTS
> INJECTION_POINT("my-inj-pt", RelationGetRelationName(rel));
> #endif
Ahah, nice. You'd probably want to schema-qualify or database-quality
that anyway.
> Worth noting, the condition types were changed to bit flags since
> we may need to combine conditions such as local injection point
> and string.
It's definitely more useful to allow combinations of them in an AND
fashion (if two conditions are defined then check both, and allow the
point to run if both conditions pass). Just to say that what you are
doing looks sensible for me. And I find that pretty cool for its
simplicity and what it could provide for future tests.
@@ -322,6 +322,7 @@ InjectionPointAttach(const char *name,
strlcpy(entry->name, name, sizeof(entry->name));
strlcpy(entry->library, library, sizeof(entry->library));
strlcpy(entry->function, function, sizeof(entry->function));
+ memset(entry->private_data, 0, INJ_PRIVATE_MAXLEN);
if (private_data != NULL)
memcpy(entry->private_data, private_data, private_data_size);
Hmm, this could be qualified as a bug, and surely it's a good practice
to clean things on attach. I'll go backpatch that.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, 2-signature.asc)
download
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Allow a condition string in an injection point
@ 2026-04-09 23:05 Sami Imseih <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Sami Imseih @ 2026-04-09 23:05 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: pgsql-hackers; Masahiko Sawada <[email protected]>; Daniil Davydov <[email protected]>
> > A follow-up to the discussion here [0], here is a patch that allows
> > for an arbitrary string in injection points to be able to apply more
> > granular filters for running an injection point. This will be useful
> > for autovacuum testing as discussed in the referenced thread,
> > and perhaps in some other places.
>
> Are the patches under discussion required for v19 or is that something
> that can wait before v20 opens for business? We have always required
> a use-case in core before adding a new API in this module, to justify
> its existence.
This is v20. One of the use-case is discussed here [1]. When testing of
autovacuum for a specific table, we need a way to run the injection point
for that table only, else we end up running the point it for all tables. This
is especially true for check-world where other non-related tables are
being autovacuumed. So this gives more granular control.
> > Worth noting, the condition types were changed to bit flags since
> > we may need to combine conditions such as local injection point
> > and string.
>
> It's definitely more useful to allow combinations of them in an AND
> fashion (if two conditions are defined then check both, and allow the
> point to run if both conditions pass). Just to say that what you are
> doing looks sensible for me. And I find that pretty cool for its
> simplicity and what it could provide for future tests.
yes, I did test with some other new hypothetical condition in the future and
it was simple to plug it in to the code at that point.
> @@ -322,6 +322,7 @@ InjectionPointAttach(const char *name,
> strlcpy(entry->name, name, sizeof(entry->name));
> strlcpy(entry->library, library, sizeof(entry->library));
> strlcpy(entry->function, function, sizeof(entry->function));
> + memset(entry->private_data, 0, INJ_PRIVATE_MAXLEN);
> if (private_data != NULL)
> memcpy(entry->private_data, private_data, private_data_size);
>
> Hmm, this could be qualified as a bug, and surely it's a good practice
> to clean things on attach. I'll go backpatch that.
It did not matter before this new condition type since the private_data
was never NULL. Not the case with this patch, as it caused inection_points test
regressions with this change. So, yeah, it's a latent bug.
--
Sami
[1] [https://www.postgresql.org/message-id/CAA5RZ0tExiffcu7qvrUbpq_qqz%3DzCD2aJ5_Qigo6eP2kgTx3eQ%40mail.g...]
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-04-09 23:05 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-04-09 18:02 Allow a condition string in an injection point Sami Imseih <[email protected]>
2026-04-09 22:38 ` Michael Paquier <[email protected]>
2026-04-09 23:05 ` Sami Imseih <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox