> > >> I think the testing discussion should be moved to a different thread. > > >> What do you think? > > > See v4. > > > > > > 0001 deals with reporting queryId in exec_execute_message and > > > exec_bind_message. > > > 0002 deals with reporting queryId after a cache invalidation. > > > > > > There are no tests as this requires more discussion in a separate > > > thread(?) > > At first, these patches look good. > > But I have a feeling of some mess here: > > queryId should be initialised at the top-level query. At the same time, > > the RevalidateCachedQuery routine can change this value in the case of > > the query tree re-validation. > > You can say that this routine can't be called from a non-top-level query > > right now, except SPI. Yes, but what about extensions or future usage?
> This is a valid point. RevalidatePlanCache is forcing a > new queryId to be advertised ( 'true' as the second argument to > pgstat_report_query_id) . This means, > v4-0002-Report-new-queryId-after-plancache-re-validation.patch > will result in a non top-level queryId being advertised. An idea would be to add bool field called force_update_qid to CachedPlanSource, and this field can be set to 'true' after a call to CreateCachedPlan. RevalidateCachedQuery will only update the queryId if this value is 'true'. For now, only exec_parse_message will set this field to 'true', but any caller can decide to set it to 'true' if there are other cases in the future. What do you think? -- Sami
