Re: Allow a condition string in an injection point - Mailing list pgsql-hackers

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