On Tue, Jan 23, 2024 at 09:12:25AM +0200, Heikki Linnakangas wrote:
> I can see that being useful in general. It requires a bespoken callback
> function, though. One nice thing about this test was that I didn't need to
> write a new C module, only that snippet at the injection point and some SQL.
> In more complicated tests a new C module is warranted, but I think a lot of
> tests - like this gin test - can be written without it.
FWIW, I don't mind if src/test/modules/injection_points/ is used as a
single point across the tree as a library storing a large set of
callbacks. All these don't have to be generic, and I think that we
should remain a maximum flexible for bug fixing and advanced testing.
I'm OK to leave the call up to the committer because it comes down to
invasiveness in src/backend/ vs invasiveness in the number of
callbacks or the logic inside the callbacks (sometimes it makes sense
to me to also complicate more the backend), and I'm OK with what you
are doing here for this thread.
> Perhaps if the argument was uint64 instead of a pointer, so that the
> 'injection_point' module could contain ready-made functions that do things
> with the argument.
I am not exactly sure to understand what you mean here.
> Overall I feel we should not bother with injection point arguments for now,
> though. Let's wait until we have a few more tests that would benefit from
> that, and then we'll have more information on what the ideal interface would
> look like.
Yeah, I am not sure if that's worth having for now, but that's always
good to keep in mind that the infra can be extended to handle such
cases even for stable branches.
--
Michael