Hi, On 2019-08-01 22:49:48 +0200, Julien Rouhaud wrote: > On Thu, Aug 1, 2019 at 8:36 PM Andres Freund <and...@anarazel.de> wrote: > > > > On 2019-08-01 14:20:46 -0400, Robert Haas wrote: > > > However, I think that the fact that this patch adds 15 new calls to > > > pg_atomic_write_u64(&MyProc->queryId, ...) is probably not a good > > > sign. It seems like we ought to be able to centralize it better than > > > that. > > > > +1 > > Unfortunately I didn't find a better way to do that. Since you can > have nested execution, I don't see how to avoid adding extra code in > every parts of query execution.
At least my +1 is not primarily about the number of sites that need to handle queryid changes, but that they all need to know about the way the queryid is stored. Including how atomicity etc is handled. That knowledge should be in one or two places, not more. In a file where that knowledge makes sense. I'm *also* concerned about the number of places, as that makes it likely that some have been missed/new ones will be introduced without the queryid handling. But that wasn't what I was referring to above. I'm actually quite unconvinced that it's sensible to update the global value for nested queries. That'll mean e.g. the log_line_prefix and pg_stat_activity values are most of the time going to be bogus while nested, because the querystring that's associated with those will *not* be the value that the queryid corresponds to. elog.c uses debug_query_string to log the statement, which is only updated for top-level queries (outside of some exceptions like parallel workers for parallel queries in a function or stuff like that). And pg_stat_activity is also only updated for top level queries. Greetings, Andres Freund
