Hello,

At Wed, 1 Jul 2026 21:51:30 +0200, Pavel Stehule <[email protected]> 
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


Reply via email to