Re: [BUG] pg_stat_statements and extended query protocol - Mailing list pgsql-hackers

From Drouvot, Bertrand
Subject Re: [BUG] pg_stat_statements and extended query protocol
Date
Msg-id 3159c384-fef2-2d5f-bfc0-44b985169389@gmail.com
Whole thread Raw
In response to Re: [BUG] pg_stat_statements and extended query protocol  (Michael Paquier <michael@paquier.xyz>)
List pgsql-hackers
Hi,

On 3/2/23 8:27 AM, Michael Paquier wrote:
> On Wed, Jan 25, 2023 at 11:22:04PM +0000, Imseih (AWS), Sami wrote:
>> Doing some work with extended query protocol, I encountered the same
>> issue that was discussed in [1]. It appears when a client is using
>> extended query protocol and sends an Execute message to a portal with
>> max_rows, and a portal is executed multiple times,
>> pg_stat_statements does not correctly track rows and calls.
> 
> Well, it is one of these areas where it seems to me we have never been
> able to put a definition on what should be the correct behavior when
> it comes to pg_stat_statements.  Could it be possible to add some
> regression tests using the recently-added \bind command and see how
> this affects things?  I would suggest splitting these into their own
> SQL file, following an effort I have been doing recently for the
> regression tests of pg_stat_statements.  It would be good to know the
> effects of this change for pg_stat_statements.track = (top|all), as
> well.
> 
> @@ -657,7 +657,9 @@ typedef struct EState
>   
>          List       *es_tupleTable;      /* List of TupleTableSlots */
>   
> -       uint64          es_processed;   /* # of tuples processed */
> +       uint64          es_processed;   /* # of tuples processed at the top level only */
> +       uint64          es_calls;       /* # of calls */
> +       uint64          es_total_processed; /* total # of tuples processed */
> 
> So the root of the logic is here.  Anything that makes the executor
> structures larger freaks me out, FWIW, and that's quite an addition.
> --
> Michael

I wonder if we can't "just" make use of the "count" parameter passed to the
ExecutorRun_hook.

Something like?

- Increment a "es_total_processed" counter in pgss based on the count received in pgss_ExecutorRun()
- In pgss_ExecutorEnd(): substract the last count we received in pgss_ExecutorRun() and add
queryDesc->estate->es_processed?(we'd
 
need to be able to distinguish when we should apply this rule or not).

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com



pgsql-hackers by date:

Previous
From: Richard Guo
Date:
Subject: Re: Assert failure of the cross-check for nullingrels
Next
From: Peter Eisentraut
Date:
Subject: Re: Allow tests to pass in OpenSSL FIPS mode