čt 2. 7. 2026 v 5:01 odesílatel Kyotaro Horiguchi <[email protected]> napsal:
> 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. > Unfortunately EnableQueryId is not reversible. So when users force EnableQueryId by non-empty log_queryids, then can be confused when trying to set empty log_queryid, but the queryid will be computed until the end of session. If I can switch queryid computing to between on, off, then this can be good idea, but becase I can only enable computing, then I don't like it too much. > > 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. > do you think some like if (!queryid_filter || log_filter) && (msec >= auto_explain_log_min_duration)) ?? > Regards, > > -- > Kyotaro Horiguchi > NTT Open Source Software Center >
