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
signature.asc
Description: PGP signature
