Hi,
On Thu, Apr 09, 2026 at 06:05:02PM -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.
>
> 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.
+1 for the idea and the use case mentioned above makes sense to me.
A few comments:
=== 1
-LANGUAGE C STRICT PARALLEL UNSAFE;
+LANGUAGE C PARALLEL UNSAFE;
and then
{
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));
As STRICT has been removed, I think that makes sense to also check PG_ARGISNULL
on 0 and 1 otherwise PG_GETARG_TEXT_PP dereferences a NULL.
=== 2
- if (!injection_point_allowed(condition))
+ if (!injection_point_allowed(condition, arg))
I think arg should be cast to char * (like injection_error() and
injection_notice()
do).
=== 3
+ 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)
));
+
Maybe this should be consistent with existing ones like:
elog(ERROR, "injection point name %s too long (maximum of %u characters)",
name, INJ_NAME_MAXLEN - 1);
=== 4
+ /* 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;
I think we should reject empty condition strings at attach time: when the string
is empty, condition->str[0] == '\0' is true, so the injection point can never
fire regardless of arg.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
