| From: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> |
|---|---|
| To: | pavel(dot)stehule(at)gmail(dot)com |
| Cc: | zsolt(dot)parragi(at)percona(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: proposal - queryid can be used as filter for auto_explain |
| Date: | 2026-07-02 03:01:44 |
| Message-ID: | 20260702.120144.1521574706549298278.horikyota.ntt@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hello,
At Wed, 1 Jul 2026 21:51:30 +0200, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote in
> > Separately from that, I don't think auto_explain should enable query
> > ID calculation when it is not actually going to use it. Instead, I
> > wonder whether this should be done in assign_log_queryids(). This
> > would naturally tie enabling query ID calculation to the state of the
> > GUC, including configuration reloads, as well as session-local use via
> > LOAD, while avoiding enabling it in sessions that never use
> > auto_explain.
> >
>
> This is a hard question - I am not sure if one GUC can force a second GUC.
I think there may be a misunderstanding.
I was not suggesting that one GUC should force another GUC. My point
was about when auto_explain calls EnableQueryId(), not about changing
the value of compute_query_id.
Instead of requesting query ID calculation unconditionally when the
module is loaded, I was wondering whether EnableQueryId() could be
called from assign_log_queryids(), so that auto_explain only requests
query ID calculation when it is actually going to use it.
> Please, check Jakub Wartak comment - I am afraid to raise log when queryid
> is not calculated and queryid_filter is not empty. I am afraid to force
> queryid calculation from assigning_queryids_filter. So the implemented
> design is the last way. Instead forcing queryid calculation in
> queryids_filter assignment is better generally not forcing queryid
> calculation.
I think that is a slightly different issue.
I was not suggesting that queryids_filter itself should force query ID
calculation, nor that we should emit a warning when query ID is not
available.
My point was that, if log_queryids is the option that makes
auto_explain use the query ID, then auto_explain could call
EnableQueryId() when that option is enabled. In that case, the
situation where queryids_filter is set but no query ID is calculated
would not be introduced by this change; if log_queryids is off,
auto_explain is not going to use the query ID anyway.
> > - if (msec >= auto_explain_log_min_duration)
> > + if (log_filter && msec >= auto_explain_log_min_duration)
> >
> > I think the overall decision logic for deciding whether to log a plan
> > could be reorganized to avoid unnecessary query ID lookups.
> >
>
> when queryids_filter will be empty, then log_filter will be true.
>
> Anytime, the user can set compute_queryid to OFF. My idea is - it should
> work by default, and when the user doesn't want it, then he can explicitly
> disable it.
Yes, that behavior makes sense. I was not suggesting changing the
meaning of an empty queryids_filter.
My comment was about the ordering of the checks in the proposed patch.
If queryids_filter is empty, no query ID lookup is needed to decide
whether the query passes the filter. Likewise, if the duration check
fails, the filter result does not matter. So I was wondering whether
the decision logic could be arranged to avoid query ID lookups in
those cases.
Regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
| From | Date | Subject | |
|---|---|---|---|
| Next Message | jian he | 2026-07-02 03:23:10 | Re: implement CAST(expr AS type FORMAT 'template') |
| Previous Message | Peter Smith | 2026-07-02 02:58:48 | DOCS - Clarify that REFRESH SEQUENCES might be using stale publication data |