On Mon, Apr 07, 2025 at 06:11:06PM -0700, Lukas Fittl wrote:
> However, since we're basically at feature freeze (and it seems unlikely
> this gets into 18), I have a quick alternate proposal:
>
> What if, for 18, we just make DoJumble exported to be used by plugins?
Yes, I am not planning to rush
> What if, for 18, we just make DoJumble exported to be used by plugins?
We will need to export InitJumble as well, but DoJumble calls
_jumbleNode which only includes queryjumblefuncs.switch.c,
so I don't think that will be enough for Plan.
--
Sami Imseih
Amazon Web Services (AWS)
Whilst rebasing the pg_stat_plans work on top of this, I realized that we
should probably make this a conditional instead of an assert - if you are
jumbling a tree that contains expressions you'd want to be able to jumble a
node that has a location (but don't record it). Attached v4 that modifies
i
On Thu, Mar 20, 2025 at 09:46:55PM -0400, Sami Imseih wrote:
> * For the same reasons as the query identifiers (see above),
>
> but, I went ahead and commented it similar to how we document
> pgstat_report_query_id and pgstat_get_my_query_id routines.
> attached is v2-0001
Looks mostly OK from he
On 3/25/25 00:47, Sami Imseih wrote:
I know it was mentioned above by both Michael and Andrei that
AppendJumble should not be exposed. I am not sure I agree with
that. I think it should be exposed, along with
JUMBLE_FIELD, JUMBLE_FIELD_SINGLE and JUMBLE_STRING
and _jumbleList.
It would be helpful
> I reviewed the patch and I only have one comment. I still think
> we need an Assert inside RecordConstantLocation to make sure clocations
> is allocated if the caller intends to record constant locations.
>
> RecordConstLocation(JumbleState *jstate, int location, bool squashed)
> {
> + Assert(jst
On Mon, Mar 31, 2025 at 1:28 PM Lukas Fittl wrote:
>
> On Tue, Mar 25, 2025 at 8:12 PM Sami Imseih wrote:
>>>
>>> So this comes down to forking the Postgres code to do the job. What I
>>> had in mind was a slightly different flow, where we would be able to
>>> push custom node attributes between
On Tue, Mar 25, 2025 at 8:12 PM Sami Imseih wrote:
> So this comes down to forking the Postgres code to do the job. What I
>> had in mind was a slightly different flow, where we would be able to
>> push custom node attributes between the header parsing and the
>> generation of the extension code
Hi! I started reviewing it and noticed that your code repeated this
cycle maybe it would be better to put it in a separate function, for
example in the form of a name like "analyze_stmts"?
or is it possible to create a macro for them?
@@ -2016,6 +2017,17 @@ exec_bind_message(StringInfo input_m
>
> So this comes down to forking the Postgres code to do the job. What I
> had in mind was a slightly different flow, where we would be able to
> push custom node attributes between the header parsing and the
> generation of the extension code. If we do that, there would be no
> need to fork the
On Tue, Mar 25, 2025 at 04:23:15PM -0500, Sami Imseih wrote:
> On 3/25/25 00:47, Sami Imseih wrote:
>> 1. Check out the upstream Postgres source for the given version I'm
>> generating jumble code for
>> 2. Modify the headers as needed to have the node attributes I want
>> 3. Call the gen_node_sup
> On 3/25/25 00:47, Sami Imseih wrote:
> > I know it was mentioned above by both Michael and Andrei that
> > AppendJumble should not be exposed. I am not sure I agree with
> > that. I think it should be exposed, along with
> > JUMBLE_FIELD, JUMBLE_FIELD_SINGLE and JUMBLE_STRING
> > and _jumbleList.
On Tue, Mar 25, 2025 at 12:13 AM Michael Paquier
wrote:
> On Mon, Mar 24, 2025 at 06:47:59PM -0500, Sami Imseih wrote:
> > I know it was mentioned above by both Michael and Andrei that
> > AppendJumble should not be exposed. I am not sure I agree with
> > that. I think it should be exposed, along
On Mon, Mar 24, 2025 at 06:47:59PM -0500, Sami Imseih wrote:
> I know it was mentioned above by both Michael and Andrei that
> AppendJumble should not be exposed. I am not sure I agree with
> that. I think it should be exposed, along with
> JUMBLE_FIELD, JUMBLE_FIELD_SINGLE and JUMBLE_STRING
> and
> On Mon, Mar 24, 2025 at 02:36:05PM -0500, Sami Imseih wrote:
> >> Ideally we can also land the jumble funcs work in the other thread
> >> to allow extensions to re-use the
> >> in-core logic for jumbling expressions found in plan node trees.
> >
> > IIUC, there should be a refactor I am working o
On Mon, Mar 24, 2025 at 02:36:05PM -0500, Sami Imseih wrote:
>> Ideally we can also land the jumble funcs work in the other thread
>> to allow extensions to re-use the
>> in-core logic for jumbling expressions found in plan node trees.
>
> IIUC, there should be a refactor I am working on at this
> Ideally we can also land the jumble funcs work in the other thread to allow
> extensions to re-use the
> in-core logic for jumbling expressions found in plan node trees.
IIUC, there should be a refactor I am working on at this moment to make that
possible [0]
>> > FWIW, Lukas did start a Wiki
> Hi! I started reviewing it and noticed that your code repeated this
> cycle maybe it would be better to put it in a separate function, for
> example in the form of a name like "analyze_stmts"?
>
> or is it possible to create a macro for them?
Perhaps it may be better to stick this all in a macro
On Sun, Mar 23, 2025 at 9:43 PM Michael Paquier wrote:
> So I've applied the patch for now, to start with
> something.
>
Thanks for committing that, I think that's a great starting point for 18!
Ideally we can also land the jumble funcs work in the other thread to allow
extensions to re-use the
On Sat, Mar 22, 2025 at 10:22:37PM -0500, Sami Imseih wrote:
> I think plan_node_id is probably the least controversial because that value
> comes straight from core, and different extensions cannot have their own
> interpretation of what that value could be.
Depends. An extension can plug in wha
On Sun, Mar 23, 2025 at 04:30:04PM +0100, Andrei Lepikhov wrote:
> So, it may be not an issue in a cloud provider predefined installations, but
> a headache for custom configurations.
Sure, I mean, but it's not really related to the issue discussed on
this thread, so..
It sounds like we could imp
On 3/23/25 15:56, Sami Imseih wrote:
Hmm, queryId generation code lies in the core and we already came to
terms that this field has only a statistical purpose. Do you want to
commit planId generation code?
But, extensions don't necessarily need to rely on the core queryId. They
can generate the
> > but the opposite may be
> > not necessarily true: a query ID could be associated with multiple
> > plan patterns (aka multiple plan IDs). What this offers is that we
> > know about which plan one query is currently using in live, for a
> > given query ID.
> Okay, as I've said before, it seems
On 3/23/25 04:22, Sami Imseih wrote:
On Sat, Mar 22, 2025 at 11:50:06PM +0100, Andrei Lepikhov wrote:
planId actually looks less controversial than queryId or plan_node_id. At
the same time, it is not free from the different logic that extensions may
incorporate into this value: I can imagine, f
On 3/23/25 01:01, Michael Paquier wrote:
On Sat, Mar 22, 2025 at 11:50:06PM +0100, Andrei Lepikhov wrote:
planId actually looks less controversial than queryId or plan_node_id. At
the same time, it is not free from the different logic that extensions may
incorporate into this value: I can imagin
> On Sat, Mar 22, 2025 at 11:50:06PM +0100, Andrei Lepikhov wrote:
> > planId actually looks less controversial than queryId or plan_node_id. At
> > the same time, it is not free from the different logic that extensions may
> > incorporate into this value: I can imagine, for example, the attempt of
On 3/22/25 06:48, Michael Paquier wrote:
On Fri, Mar 21, 2025 at 09:25:24AM -0400, Sami Imseih wrote:
planner() is the sole place in the core code where the planner hook
can be called. Shouldn't we have at least a call to
pgstat_report_plan_id() after planning a query? At least that should
be
On Sat, Mar 22, 2025 at 11:50:06PM +0100, Andrei Lepikhov wrote:
> planId actually looks less controversial than queryId or plan_node_id. At
> the same time, it is not free from the different logic that extensions may
> incorporate into this value: I can imagine, for example, the attempt of
> uniqu
On Fri, Mar 21, 2025 at 09:25:24AM -0400, Sami Imseih wrote:
>> planner() is the sole place in the core code where the planner hook
>> can be called. Shouldn't we have at least a call to
>> pgstat_report_plan_id() after planning a query? At least that should
>> be the behavior I'd expect, where a
> In exec_bind_message(), the comment at the top of PortalDefineQuery()
> tells to not put any code between this call and the GetCachedPlan()
> that could issue an error. pgstat_report_plan_id() is OK, but I'd
> rather play it safe and set the ID once the portal is defined, based
> on portal->stmt
> > This adds a planId to pg_stat_get_activity ( not pg_stat_activity ).
> > An extension
> > can offer its own view, similar to pg_stat_activity, which can include
> > planId.
> >
> > Note that there are no documentation updates required here as we don't
> > have per-field documentation for pg_st
Hi,
Thank you for the patches. They're very important for many extensions.
I've debugged them using simple extensions pg_stat_statements and
auto_explain, specifically checking cases involving PlannedStmt
and pg_stat_get_activity - , and haven't encountered any issues so far.
However, I have
On Thu, Feb 20, 2025 at 05:03:19PM -0600, Sami Imseih wrote:
> v2-0001:
>
> This adds a planId to pg_stat_get_activity ( not pg_stat_activity ).
> An extension
> can offer its own view, similar to pg_stat_activity, which can include planId.
>
> Note that there are no documentation updates require
On 18.03.2025 14:46, Ilia Evdokimov wrote:
However, I have a question: what value should planid have when we
execute the standard planner without using a planner_hook? For
example, if pg_stat_statementsis disabled, would planid default to
zero? If zero is the expected default, should we exp
I put together patches to do as is being proposed.
v1-0001:
1. Adds a planId field in PlannedStmt
2. Added an st_plan_id fields in PgBackendStatus
3. APIs to report and to retrieve a planId to PgBackendStatus
An extension is able to set a planId in PlannedStmt directly,
and while they can do the
On Sun, Feb 16, 2025 at 5:34 PM Michael Paquier wrote:
>
> On Sat, Feb 15, 2025 at 10:29:41AM +0100, Andrei Lepikhov wrote:
> > I have already implemented it twice in different ways as a core patch.
> > In my projects, we need to track queryId and plan node ID for two reasons:
>
> Are these availa
On Sat, Feb 15, 2025 at 10:29:41AM +0100, Andrei Lepikhov wrote:
> I have already implemented it twice in different ways as a core patch.
> In my projects, we need to track queryId and plan node ID for two reasons:
Are these available in the public somewhere or is that simply what
Sami is proposin
On 14/2/2025 08:21, Michael Paquier wrote:
On Thu, Feb 13, 2025 at 11:10:27AM -0600, Sami Imseih wrote:
I don't think direct setting of values is a good idea. We will need an API
similar to pgstat_report_query_id which ensures we are only reporting top
level planIds -and- in the case of multiple
On Thu, Feb 13, 2025 at 11:10:27AM -0600, Sami Imseih wrote:
> I don't think direct setting of values is a good idea. We will need an API
> similar to pgstat_report_query_id which ensures we are only reporting top
> level planIds -and- in the case of multiple extensions with the capability
> to set
Thanks for the feedback!
> I think that makes sense and then the idea would be later on to move to
> something
> like 5fd9dfa5f50, but for the "planId": is my understanding correct?
correct. This is adding infrastructure to eventually have an in-core
planId; but in the
meanwhile give extensions
Hi,
On Wed, Feb 12, 2025 at 05:44:20PM -0600, Sami Imseih wrote:
> Meanwhile, existing extensions like pg_stat_monitor [3] compute a planId and
> store the plan text, but they lack a way to expose planId in pg_stat_activity.
> This limits their usefulness, as identifying top or long-running plans
Hi,
A patch by Lukas Fittl [1] introduces the pg_stat_plans extension, which
exposes execution plans along with execution statistics. As part of this work,
a new infrastructure is being proposed to compute a plan identifier (planId),
which is exposed in both pg_stat_plans and pg_stat_activity.
Ex
42 matches
Mail list logo
