Re: Add privileges test for pg_stat_statements to improve coverage - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Add privileges test for pg_stat_statements to improve coverage
Date
Msg-id Zp72jo7-8TKcUiuB@paquier.xyz
Whole thread Raw
In response to Re: Add privileges test for pg_stat_statements to improve coverage  (Fujii Masao <masao.fujii@oss.nttdata.com>)
Responses Re: Add privileges test for pg_stat_statements to improve coverage
List pgsql-hackers
On Tue, Jul 23, 2024 at 01:51:19AM +0900, Fujii Masao wrote:
> +SELECT query, calls, rows FROM pg_stat_statements
> +  WHERE queryid IS NULL ORDER BY query COLLATE "C";
>
> Shouldn't we also include calls and rows in the ORDER BY clause?
> Without this, if there are multiple records with the same query
> but different calls or rows, the query result might be unstable.
> I believe this is causing the test failure reported by
> he PostgreSQL Patch Tester.
>
> http://cfbot.cputube.org/
> https://cirrus-ci.com/task/4533613939654656

+SELECT query, calls, rows FROM pg_stat_statements
+  WHERE queryid IS NULL ORDER BY query COLLATE "C";
+          query           | calls | rows
+--------------------------+-------+------
+ <insufficient privilege> |     1 |    1
+ <insufficient privilege> |     1 |    1
+ <insufficient privilege> |     1 |    3
+(3 rows)

I'd recommend to add a GROUP BY on calls and rows, with a
count(query), rather than print the same row without the query text
multiple times.

+-- regress_stats_user2 can read query text and queryid
+SET ROLE regress_stats_user2;
+SELECT query, calls, rows FROM pg_stat_statements
+  WHERE queryid <> 0 ORDER BY query COLLATE "C";
+                       query                        | calls | rows
+----------------------------------------------------+-------+------
+ SELECT $1 AS "ONE"                                 |     1 |    1
+ SELECT $1+$2 AS "TWO"                              |     1 |    1
+ SELECT pg_stat_statements_reset() IS NOT NULL AS t |     1 |    1
+ SELECT query, calls, rows FROM pg_stat_statements +|     1 |    1
+   WHERE queryid <> $1 ORDER BY query COLLATE "C"   |       |
+ SELECT query, calls, rows FROM pg_stat_statements +|     1 |    3
+   WHERE queryid <> $1 ORDER BY query COLLATE "C"   |       |

We have two entries here with the same query and the same query ID,
because they have a different userid.  Shouldn't this query reflect
this information rather than have the reader guess it?  This is going
to require a join with pg_authid to grab the role name, and an ORDER
BY on the role name.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: Add on_error and log_verbosity options to file_fdw
Next
From: Michael Paquier
Date:
Subject: Re: Vacuum ERRORs out considering freezing dead tuples from before OldestXmin