On Thu, 23 Mar 2023 09:33:16 +0100 "Drouvot, Bertrand" <bertranddrouvot...@gmail.com> wrote:
> Hi, > > On 3/22/23 10:35 PM, Imseih (AWS), Sami wrote: > >> What about using an uint64 for calls? That seems more appropriate to me > >> (even if > >> queryDesc->totaltime->calls will be passed (which is int64), but that's > >> already > >> also the case for the "rows" argument and > >> queryDesc->totaltime->rows_processed) > > > > That's fair > > > > > >> I'm not sure it's worth mentioning that the new counters are "currently" > >> used with the ExecutorRun. > > > > Sure, I suppose these fields could be used outside of ExecutorRun. Good > > point. > > > > > >> Also, I wonder if "rows" (and not rows_processed) would not be a better > >> naming. > > > > Agree. > > > > I went with rows_processed initially, since it was accumulating > > es_processed, > > but as the previous point, this instrumentation could be used outside of > > ExecutorRun. > > > > v3 addresses the comments. I wonder that this patch changes the meaning of "calls" in the pg_stat_statement view a bit; previously it was "Number of times the statement was executed" as described in the documentation, but currently this means "Number of times the portal was executed". I'm worried that this makes users confused. For example, a user may think the average numbers of rows returned by a statement is given by rows/calls, but it is not always correct because some statements could be executed with multiple portal runs. Although it might not big issue to users, I think it is better to add an explanation to the doc for clarification. Regards, Yugo Nagata > > > > Thanks! LGTM and also do confirm that, with the patch, the JDBC test does > show the correct results. > > That said, not having a test (for the reasons you explained up-thread) > associated with the patch worry me a bit. > > But, I'm tempted to say that adding new tests could be addressed separately > though (as this patch looks pretty straightforward). > > Regards, > > -- > Bertrand Drouvot > PostgreSQL Contributors Team > RDS Open Source Databases > Amazon Web Services: https://aws.amazon.com > > -- Yugo NAGATA <nag...@sraoss.co.jp>
