Re: Allow a condition string in an injection point

> > 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.gmail.com]