Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Zero-fill private_data when attaching an injection point

  1. Allow a condition string in an injection point

    Sami Imseih <samimseih@gmail.com> — 2026-04-09T18:02:37Z

    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
    
  2. Re: Allow a condition string in an injection point

    Michael Paquier <michael@paquier.xyz> — 2026-04-09T22:38:38Z

    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
    
  3. Re: Allow a condition string in an injection point

    Sami Imseih <samimseih@gmail.com> — 2026-04-09T23:05:02Z

    > > 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]