Hi,

While testing the new feature "Show sizes of FETCH queries as constants in 
pg_stat_statements”, I found a problem where query string shown depends on 
order of FETCH statements.

This is a simple repro:

Setup:
```
select pg_stat_statements_reset();
begin;
declare c cursor for select g from generate_series(1, 10) g;
```

If FETCH without a count is executed first:
```
evantest=*# fetch c;
 g
---
 1
(1 row)

evantest=*# fetch 2 c;
 g
---
 2
 3
(2 rows)

evantest=*# commit;
COMMIT
evantest=#
evantest=#  select calls, query from pg_stat_statements where query like 
‘fetch%c%';
 calls |  query
-------+-----------
     2 | fetch c
(1 row)
```

The query text is shown as the unnormalized "fetch c”.

But if FETCH with a count is executed first:
```
evantest=*# fetch 2 c;
 g
---
 1
 2
(2 rows)

evantest=*# fetch c;
 g
---
 3
(1 row)

evantest=*# commit;
COMMIT
evantest=#
evantest=# select calls, query from pg_stat_statements where query like 
'fetch%c%';
 calls |   query
-------+------------
     2 | fetch $1 c
(1 row)
```

Then the query text is shown as the normalized “fetch $1 c”. This seems 
incorrect to me, because the representative query text should not depend on the 
execution order of FETCH statements.

The attached patch tries to fix this by adding a query_normalized flag to 
pgssEntry, which records whether the stored representative query text is 
already normalized. With this flag, if FETCH c is executed first and stores an 
unnormalized query string, a later FETCH 2 c can replace it with the normalized 
query string.

One part of the implementation that I am not fully satisfied with is that I 
added a new parameter to pgss_store() to opt-in to the replacement logic only 
for FETCH statements. Without that restriction, SET SESSION AUTHORIZATION is 
broken: SET SESSION AUTHORIZATION 'r1' and SET SESSION AUTHORIZATION 'r2' are 
not combined into one pg_stat_statements entry, so blindly applying this 
replacement logic more broadly would be wrong.

I am not sure whether it is better to opt-in only for FETCH, or to apply the 
logic more broadly and explicitly opt-out cases such as SET SESSION 
AUTHORIZATION. Comments and suggestions are welcome.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/




Attachment: v1-0001-Fix-pg_stat_statements-display-of-normalized-FETC.patch
Description: Binary data

Reply via email to