Thread: Improve explicit cursor handling in pg_stat_statements

Improve explicit cursor handling in pg_stat_statements

From
Sami Imseih
Date:
Hi hackers,

I recently looked into a workload that makes heavy use of explicit cursors,
and I found that pg_stat_statements can become a bottleneck. The
application in question declares hundreds of cursors, and for each one,
performs many FETCH and MOVE operations with varying fetch sizes.

As a result, pg_stat_statements ends up overwhelmed by the deallocation
(and garbage collection) of DECLARE CURSOR, FETCH, and MOVE entries.
Each of these is associated with a unique queryId, which leads to bloated
entries with limited diagnostic value.

Other issues:

1. FETCH/MOVE statements don't really help much in laying blame on a specific
query. the DECLARE CURSOR statement could have been evicted in
pg_stat_statements
by that point or a similar cursor name is pointing to a different query.

Also, FETCH doesn't aggregate for the same cursor — e.g., FETCH 10 c1 and
FETCH 20 c1 show up as separate entries.

2. DECLARE CURSOR doesn't provide execution stats for the underlying SQL.
Enabling pg_stat_statements.track = 'all' can expose the underlying SQL,
but adds overhead.There’s also a bug: the toplevel column for the underlying
query is still marked as "t", even though you must set track "all" to see it.

Based on this, I propose the following improvements:

1. Better normalization of cursor utility commands:

2. Normalize the cursor name in CLOSE.

3. Normalize fetch/move sizes in FETCH and MOVE. Users can use the rows and
calls columns to derive average fetch size. Ideally I would want to
normalize the
cursor name and generate the queryId f the FETCH statement based on the
underlying query, but that is not possible to do that post parsing.

(The above normalizations of these utility statements will reduce the bloat.)

4. Track the underlying query of a cursor by default, even when
pg_stat_statements.track_utility = off.

I’ve attached two patches that implement this. Here's a quick example:

```
begin;
declare cs1 cursor for select from pg_class;
declare cs2 cursor for select from pg_class;
fetch 10 cs1;
fetch 20 cs1;
fetch 10 cs1;
fetch 10 cs2;
close cs1;
close cs2;
declare cs1 cursor for select from pg_attribute;
SELECT calls, rows, query, toplevel FROM pg_stat_statements ORDER BY
query COLLATE "C";
commit;
```

current state:
```
postgres=*# SELECT calls, rows, query, toplevel FROM
pg_stat_statements ORDER BY query COLLATE "C";
 calls | rows |
query                                                   | toplevel

-------+------+-----------------------------------------------------------------------------------------------------------+----------
     1 |    1 | SELECT name    FROM pg_catalog.pg_available_extensions
  WHERE name LIKE $1 AND installed_version IS NULL+| t
       |      | LIMIT $2
                                                   |
     1 |    0 | begin
                                                   | t
     1 |    0 | close cs1
                                                   | t
     1 |    0 | close cs2
                                                   | t
     1 |    0 | create extension pg_stat_statements
                                                   | t
     1 |    0 | declare cs1 cursor for select from pg_attribute
                                                   | t
     1 |    0 | declare cs1 cursor for select from pg_class
                                                   | t
     1 |    0 | declare cs2 cursor for select from pg_class
                                                   | t
     2 |   20 | fetch 10 cs1
                                                   | t
     1 |   10 | fetch 10 cs2
                                                   | t
     1 |   20 | fetch 20 cs1
                                                   | t
(11 rows)
```

with both patches applied:
```
postgres=*# SELECT calls, rows, query, toplevel FROM
pg_stat_statements ORDER BY query COLLATE "C";
 calls | rows |                     query                      | toplevel
-------+------+------------------------------------------------+----------
     1 |    0 | begin                                          | t
     2 |    0 | close $1                                       | t
     1 |    0 | declare $1 cursor for select from pg_attribute | t
     2 |    0 | declare $1 cursor for select from pg_class     | t
     3 |   40 | fetch $1 cs1                                   | t
     1 |   10 | fetch $1 cs2                                   | t
     2 |   50 | select from pg_class                           | t
(7 rows)

postgres=*# commit;
COMMIT
```

FWIW, I raised this ~3 years ago [0], but there was not much interest. I
have seen this being a problem a few times since then that I think something
should be done about. I also was not happy with the approach I took in [0].

Looking forward to feedback!

Regards,

--
Sami Imseih
Amazon Web Services (AWS)

[0] https://www.postgresql.org/message-id/flat/203CFCF7-176E-4AFC-A48E-B2CECFECD6AA%40amazon.com

Attachment

Re: Improve explicit cursor handling in pg_stat_statements

From
Sami Imseih
Date:
I forgot to add the proper tests to the Normalize cursor utility statements.
Reattaching v2.

I also want to add that the decision to not normalize the cursor name in
the FETCH command is because it would not make sense to combine
FETCH commands for various cursors into the same entry.

Regards,

--
Sami Imseih
Amazon Web Services (AWS)

Attachment

Re: Improve explicit cursor handling in pg_stat_statements

From
Michael Paquier
Date:
On Wed, Apr 30, 2025 at 02:43:41PM -0500, Sami Imseih wrote:
> I also want to add that the decision to not normalize the cursor name in
> the FETCH command is because it would not make sense to combine
> FETCH commands for various cursors into the same entry.

- calls | rows |                         query
--------+------+-------------------------------------------------------
-     2 |    0 | CLOSE cursor_stats_1
-     2 |    0 | DECLARE cursor_stats_1 CURSOR WITH HOLD FOR SELECT $1
+ calls | rows |                       query
+-------+------+----------------------------------------------------
+     2 |    0 | CLOSE $1
+     2 |    0 | DECLARE $1 CURSOR WITH HOLD FOR SELECT $2

Hmm.  What are the workloads that you have seen as problematic?  Do
these involve cursor names generated randomly, where most of them are
similar with a random factor for the name?  Too much normalization
here would limit the amount of verbosity that we have for this area,
especially if we are dealing with query patterns that rely on few
CLOSE naming patterns spread across a couple of key queries, because
we would now know anymore about their distribution.

-     1 |    5 | FETCH FORWARD 5 pgss_cursor
-     1 |    7 | FETCH FORWARD ALL pgss_cursor
-     1 |    1 | FETCH NEXT pgss_cursor
+     1 |    0 | DECLARE $1 CURSOR FOR SELECT * FROM pgss_matv

Saying that, applying normalization for the number of FETCH looks like
a natural move.  It seems to me that we should still make a difference
with the ALL case, though?

 typedef struct ClosePortalStmt
 {
     NodeTag        type;
-    char       *portalname;        /* name of the portal (cursor) */
+    /* name of the portal (cursor) */
+    char       *portalname pg_node_attr(query_jumble_ignore);
+    ParseLoc    location pg_node_attr(query_jumble_location);
     /* NULL means CLOSE ALL */

Could it matter to make a distinction with CLOSE ALL, compared to the
case where the CLOSE statements are named?  It would be possible to
make a difference compared to the named case with an extra boolean
field, for example.  I would suggest to add some tests for CLOSE ALL
anyway; we don't have any currently.
--
Michael

Attachment

Re: Improve explicit cursor handling in pg_stat_statements

From
Sami Imseih
Date:
> Hmm.  What are the workloads that you have seen as problematic?  Do
> these involve cursor names generated randomly, where most of them are
> similar with a random factor for the name?

postgres_fdw, as an example, in which cursor name get reused
for different queries. Notice below "c1" and "c2" is reused for different
queries, so now what underlying sql is FETCH, i.e. FETCH 100 FROM c1 referring
to? v2-0001 does not help us with the FETCH problem
because as I mentioned we don't have access to the underlying sql
( and parsing is even too early to do a portal lookup to find the
underlying sql to
base the queryId on). What v2-0001 will do is at least group the DECLARE CURSOR
statements together for cursors referencing the same query and reduce the #
of entries.

```
create foreign table t2(id int) server r1;
create foreign table t1(id int) server r1;

postgres=# select * from t2, t ;
 id | id
----+----
  1 |  1
(1 row)

postgres=# select * from t, t2 ;
 id | id
----+----
  1 |  1
(1 row)
```
on the remote side
```
postgres=# select calls, query from pg_stat_statements where query like '% c%';
 calls |                              query
-------+-----------------------------------------------------------------
     1 | DECLARE c2 CURSOR FOR                                          +
       | SELECT id FROM public.t2
     2 | DECLARE c1 CURSOR FOR                                          +
       | SELECT id FROM public.t2
     3 | CLOSE c2
     3 | CLOSE c1
     2 | DECLARE c2 CURSOR FOR                                          +
       | SELECT id FROM public.t
     3 | FETCH 100 FROM c1
     3 | FETCH 100 FROM c2
     1 | DECLARE c1 CURSOR FOR                                          +
       | SELECT id FROM public.t
     2 | select calls, query from pg_stat_statements where query like $1
(9 rows)
```

> Too much normalization
> here would limit the amount of verbosity that we have for this area,
> especially if we are dealing with query patterns that rely on few
> CLOSE naming patterns spread across a couple of key queries, because
> we would now know anymore about their distribution.

The FETCH and CLOSE are already not clear to what underlying SQL
they are referring to, and there is not much chance to actually
improve that unless
we track a cursor queryId in pg_stat_statements ( at that point we can show that
FETCH or CLOSE refer to this specific cursor statement ).

--
Sami



Re: Improve explicit cursor handling in pg_stat_statements

From
Michael Paquier
Date:
On Fri, May 02, 2025 at 04:21:21PM -0500, Sami Imseih wrote:
> postgres_fdw, as an example, in which cursor name get reused
> for different queries. Notice below "c1" and "c2" is reused for different
> queries, so now what underlying sql is FETCH, i.e. FETCH 100 FROM c1 referring
> to? v2-0001 does not help us with the FETCH problem
> because as I mentioned we don't have access to the underlying sql
> ( and parsing is even too early to do a portal lookup to find the
> underlying sql to
> base the queryId on). What v2-0001 will do is at least group the DECLARE CURSOR
> statements together for cursors referencing the same query and reduce the #
> of entries.

This case relies on postgres_fdw's GetCursorNumber() that assigns a
unique number for a cursor, ensuring uniqueness per connection within
a transaction, and the counter is reset at the end of the
transactions.  So good point for this case that this hurts.  If that
holds for the most common cases where this is seen as bloating pgss,
that brings some solid ground, especially more for applications that
use many cursor numbers in long-ish transactions states done under
postgres_fdw.

I'm still slightly worried about workloads where cursor names could be
used to track some balancing of this kind of activity, for example if
cursor names are fixed on a transaction-basis depending on the
application involved, as that would mean less visibility in the
information.  Perhaps we would have more confidence by looking closer
at drivers or some ORMs that make use of cursors, seeing how the end
user would be impacted?  That seems hard to measure, though..

> The FETCH and CLOSE are already not clear to what underlying SQL
> they are referring to, and there is not much chance to actually
> improve that unless
> we track a cursor queryId in pg_stat_statements ( at that point we can show that
> FETCH or CLOSE refer to this specific cursor statement ).

I don't really have an issue for FETCH with the number as the name is
still around, but I'm equally worrying about the loss of information
for CLOSE that this new normalization would imply.  Perhaps my worries
don't have a reason to exist here and I'm just a naturally-pessimistic
being.
--
Michael

Attachment