Re: proposal - queryid can be used as filter for auto_explain

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com>
Cc: Julien Rouhaud <rjuju123(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: proposal - queryid can be used as filter for auto_explain
Date: 2026-06-18 19:47:30
Message-ID: CAFj8pRDzkpbk8SUANhrFSSqgOxMvHWgo85=DV7NbYgo16Qh+WQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi

čt 18. 6. 2026 v 13:38 odesílatel Jakub Wartak <
jakub(dot)wartak(at)enterprisedb(dot)com> napsal:

> Hi Pavel,
>
> On Wed, Jun 17, 2026 at 9:21 AM Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
> >
> > Hi
> >
> > small change - parsing queryid by usage pg_strtoint64_safe instead strtol
> >
>
> + DefineCustomStringVariable("auto_explain.log_queryids",
> + "Only
> queries with qyeryid from list will be logged.",
>
> Other than this 'qyeryid' typo I don't see anything obviously wrong. Well
> maybe if we are scared about too heavy impact of scanning too long array
> of queryids, we could enforce some max limitation there on the count
> accepted (but I'm not sure if it is worth it).
>
> + if (log_filter && msec >= auto_explain_log_min_duration)
>
> With QA hat on: maybe it would be possible to emit some warning message
> if log_queryids is set to anything and auto_explain_log_min_duration is
> at the default of -1? (because that way nothing gets logged). My first
> impression when using was If I would enable it .log_queryids would work
> out of the box, but of course due to this line of code both criteria
> need to be meet, but emitting some HINT when just .log_queryids would be
> set would be more user firendly (yes, it's covered in the docs, but people
> rarely check it).
>

Unfortunately, I don't know when this warning should be raised. If I raise
when I parse GUC, then
the warning will be raised everywhere, when users use SET or set_config
function (because,
only once GUC can be changed at one time). If I raise a warning at executor
end time, then
there is a risk of a log storm - this is what I exactly don't want.

An alternative can be implementation when log_queryids and log_min_duration
are independent constraints. Then there is not a problem that you describe.
But from
practical usage, I think current implementation is better - sometimes we
want only specific
plans longer than specified time. There can be another GUC, that can
specify the relation
between log_queryids and log_min_duration - but at this time it looks like
overengineering.

So warning at documentation looks like the lesser of two evils.

Thank you for check

Nice evening

Pavel

> Anyway, it looks like very useful addition to me.
>
> -J.
>

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Jacob Champion 2026-06-18 20:01:47 PG20 Minimum Dependency Thread
Previous Message Jeff Davis 2026-06-18 18:52:40 Re: GUC parameter ACLs and physical walsender