Feedback-ID: i0fe9450f:Fastmail
Date: Fri, 10 Apr 2026 07:38:38 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Sami Imseih <samimseih@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>,
	Masahiko Sawada <sawada.mshk@gmail.com>,
	Daniil Davydov <3danissimo@gmail.com>
Subject: Re: Allow a condition string in an injection point
Message-ID: <adgqbgfA7KaNwCIS@paquier.xyz>
References: 
 <CAA5RZ0tsGHu2h6YLnVu4HiK05q+gTE_9WVUAqihW2LSscAYS-g@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="rk5YzPsEWX5GRsCG"
Content-Disposition: inline
In-Reply-To: 
 <CAA5RZ0tsGHu2h6YLnVu4HiK05q+gTE_9WVUAqihW2LSscAYS-g@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/adgqbgfA7KaNwCIS%40paquier.xyz>
Precedence: bulk


--rk5YzPsEWX5GRsCG
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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.
>=20
> 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 !=3D 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

--rk5YzPsEWX5GRsCG
Content-Type: application/pgp-signature; name=signature.asc

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmnYKm4ACgkQnvQgOdby
QH3J0w/9ERk2+TeLdNVMQjXlM0WAfMdCCF03Pi5GIqYDOd87xDE78Q/hAMoTAMge
IfkVthfaEueBixfKBG7qKIyWmYxYHfoMawMDrmzbS+YDgsi9PL7qR/NdP86vBrNO
dhr2li2V8mlsPTJDyOio42DvYb/C3uUQptKoXRDeIZKE2bciH57lOWe6QKyMLa+V
wkty9CAiI/xHGFw8Wn9RGiXOa4U3lSTVECqHNjutXS09T+K6hGMDgddg+xUojteK
8Tqp0U226O7Aw4+P8XVLCawOV55KzBNioePNrQm1qxcjZeNvxPGB0xMEcXte/4Q0
KNPWcAi+2IrHdGGZvbf+5pAUF8tb1bXlzUASoYKVsYJgh9qktQGF5davfDW8HEYt
axpW7/MzPFbxnmANGISLGSyTPSWTdlJUDXtcbFyVlmQdcrp1FPaPy50pq33yGqRT
d0QC2jzmTQmZoRghc4egpST93m+a/G1wGWcazzF/lQvG7Nz2jsaXKoZ3o/THuxZV
2aqbGxM84ldaNyW7Rnc7wOGFXGkEAEOWAh5IMQZJJGpJEV/e5jDjdpqD0zSY5Lda
Qtjf9RbAXQWBsEbXQuDoyWh1eM9cqQ/u5yEBmuczfe9hIbKKqpKZIxme6p3F6sKx
CLbdDSSe5m1mRdLnwB/CbiZg6hX841dYydsynTZmPfq1FlPZxNM=
=TaAa
-----END PGP SIGNATURE-----

--rk5YzPsEWX5GRsCG--