On Thu, 23 Mar 2023 09:33:16 +0100
"Drouvot, Bertrand" <bertranddrouvot.pg@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 <nagata@sraoss.co.jp>
