On Thu, Jul 03, 2025 at 12:14:55PM +0530, Rahila Syed wrote:
> #include "miscadmin.h"
> +#include "nodes/execnodes.h"
> #include "nodes/pg_list.h"
> #include "nodes/value.h"
>
> Do we need to include this? I did not see any compilation error when I
> removed this.
Right, this one was not neede
On Thu, Jul 03, 2025 at 12:14:55PM +0530, Rahila Syed wrote:
>> Now for the second part with the SRF making the injection point
>> information available at SQL level. The point about adding the new
>> function in the core engine has been sticky, and I'm coming down to
>> the conclusion that I'd st
Hi Michael,
#include "miscadmin.h"
+#include "nodes/execnodes.h"
#include "nodes/pg_list.h"
#include "nodes/value.h"
Do we need to include this? I did not see any compilation error when I
removed this.
> Now for the second part with the SRF making the injection point
> information available
On Thu, Jul 03, 2025 at 08:51:56AM +0900, Michael Paquier wrote:
> This results in the second patch attached. Comments are welcome.
There has been a blip with the regression test output, so rebased.
--
Michael
From e70d1606306a9933655fbae6f8a3f472c110 Mon Sep 17 00:00:00 2001
From: Michael Pa
On Tue, Apr 15, 2025 at 08:00:50AM +0900, Michael Paquier wrote:
> On Mon, Apr 14, 2025 at 04:29:37PM +0300, Aleksander Alekseev wrote:
>> If I didn't miss anything, all SQL functions dealing with injection
>> points are gathered in injection_points extension so IMO
>> pg_get_injection_points() bel
On Sun, May 25, 2025 at 03:59:51AM +, Rustam ALLAKOV wrote:
> to match how `unicode_version` function is documented
> > Returns a string representing the version of Unicode used by ICU, if
> > the server was built with ICU support; otherwise returns
Makes sense, with the difference that the fu
The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation:tested, failed
Hi Michael,
Thank you for the patch.
I suggest to add into documentation
On Mon, Apr 28, 2025 at 10:10:09PM +0530, Rahila Syed wrote:
> Should the execution privileges on the function be restricted to a role
> like pg_monitor?
I am not sure that it would be a good thing here, forcing tests to
implement an extra step when requiring a restrictive role if this role
needs
Hi Michael,
Thank you for the updated patches.
> Would it be useful to put the logic of the above function under #define
> > USE_INJECTION_POINT. This approach would make it possible to
> > distinguish between cases where no injection points are attached and
> > instances where the build does no
On Tue, Apr 15, 2025 at 04:37:51PM +0530, Rahila Syed wrote:
> The function actually retrieves an array not a list, so the comment and
> the function name may be misleading.
> This function not only retrieves an array of injection points but also
> the number of injection points. Would it be more e
Hi,
Thank you for this enhancement to injection points.
Following are a few comments.
+ * This overestimates the allocation size, including slots that may be
+ * free, for simplicity.
+ */
+ result = palloc0(sizeof(InjectionPointData) * max_inuse);
+
The result variable name is a bit generic. H
On Mon, Apr 14, 2025 at 05:26:14PM +0500, Kirill Reshke wrote:
> Hi! I noticed you do not bump catalog version here, so i was a little
> confused with
> ```
> reshke=# SELECT name, library, function FROM pg_get_injection_points();
> ERROR: function pg_get_injection_points() does not exist
> ```
>
On Tue, Apr 15, 2025 at 02:49:24AM +, Hayato Kuroda (Fujitsu) wrote:
> Should cur_pos be uint32 instead of int? Either of them can work because
> cur_pos can be [0, 128], but it may be clearer.
>
> Apart from above, LGTM.
Sure, why not. Thanks for the review.
An important thing may be to sp
Dear Michael,
> Yes. The function could be changed to return an ERROR if the build
> does not support this option. It's more portable to return NULL if
> you have some queries that do joins. This could be used with
> pg_stat_activity and wait events for example, and some points are in
> positio
On Mon, Apr 14, 2025 at 04:29:37PM +0300, Aleksander Alekseev wrote:
> If I didn't miss anything, all SQL functions dealing with injection
> points are gathered in injection_points extension so IMO
> pg_get_injection_points() belongs there. It would be awkward to have
> it in the core considering t
On Mon, Apr 14, 2025 at 06:15:07AM +, Hayato Kuroda (Fujitsu) wrote:
> Apart from functions related with injection_points, this can be called even
> when
> postgres has been built with -Dinjection_points=false. This is intentional
> because
> this function does not have any side-effect and ju
On Mon, 14 Apr 2025 at 17:32, Michael Paquier wrote:
>
> As this is global for all libraries, that's not something I would add
> there.
Same applies for pg_stat_statements, but it is defined in extension.
Also, injection points are tests-only, so maybe no need to have this
function callable in p
Em seg., 14 de abr. de 2025 às 09:46, Ranier Vilela
escreveu:
> Hi Michael.
>
> Em dom., 13 de abr. de 2025 às 21:36, Michael Paquier
> escreveu:
>
>> Hi all,
>>
>> One thing that's been lacking in injection points is the possibility
>> to look at the state of the injection points in shared memo
Hi,
> > Also, should we define pg_get_injection_points in the injection_points
> > extension maybe?
>
> As this is global for all libraries, that's not something I would add
> there.
If I didn't miss anything, all SQL functions dealing with injection
points are gathered in injection_points extens
Dear Michael,
Thanks for creating the patch! Let me confirm two points:
Apart from functions related with injection_points, this can be called even when
postgres has been built with -Dinjection_points=false. This is intentional
because
this function does not have any side-effect and just refers
Hi all,
One thing that's been lacking in injection points is the possibility
to look at the state of the injection points in shared memory through
some SQL. This was on my tablets for some time, but I have not taken
the time to do the actual legwork.
The attached patch adds a SRF that returns a
21 matches
Mail list logo