On Wed, May 06, 2026 at 02:51:00PM +0500, Andrey Borodin wrote:
> cc to Michael:
>
> prockill_race needs to build the same InjectionPointCondition payload that
> injection_wait consumes to know which PID to block. The struct is currently
> private to injection_points.c, so the patch extracts it into a small header
> that prockill_race.c includes via a relative "../injection_points/" path.
> That works but feels non-idiomatic. Since injection_points grows organically
> to support new bug reproducers anyway, making the condition type part of its
> public header seems like a natural fit - but we are not sure the fix is
> committable as-is, so we wanted to ask before doing any more cleanup: is
> this refactor acceptable at all, and if so, would you prefer a proper
> installed header (as contrib/pg_plan_advice does) over the relative include?
I did not look at the bug fix in details, so this is a comment about
the structure of the test.
+#include "../injection_points/injection_point_condition.h"
Hmm. I would not see a problem in just moving all that to the module
injection_points instead, and keep it there, including your TAP test.
Noah has done something similar for its removable_cutoff() business,
and we are living well with it. One issue with the structure you are
proposing is that I suspect that it makes some installcheck scenarios
more iffy to deal with. More callbacks in the test module is fine.
--
Michael
