Subject: Re: pg_stat_statements and "IN" conditions

From: Álvaro Herrera <[email-masked]>
Date: Tue, 11 Feb 2025 13:14:52 +0100

Hello

I noticed something that surprised me at first, but on further looking
it should have been obvious: setting pg_stat_statements.query_id_const_merge
affects the query ID for all readers of it, not just pg_stat_statement.
This is good because it preserves the property that pg_stat_activity
entries can be matched to pg_stat_statement entries by query_id.

Looking to commit 0001 soon.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"On the other flipper, one wrong move and we're Fatal Exceptions"
(T.U.X.: Term Unit X  - http://www.thelinuxreview.com/TUX/)

---

From: Sami Imseih <[email-masked]>
Date: Tue, 11 Feb 2025 10:49:59 -0600

I have only looked at 0001, but I am wondering why
query_id_const_merge is a pg_stat_statements GUC
rather than a core GUC?

The dependency of pg_stat_statements to take advantage
of this useful feature does not seem right.

For example if the user does not have pg_stat_statements enabled,
but are sampling top queryId from pg_stat_activity, they will
likely want this merge behavior to build meaningful database
load graphs.

Other extensions that consume queryIds may also want this
behavior without needing to enable pg_stat_statements.

Also, we have compute_query_id as a core parameter, this
new guc will become an option for how to compute a queryId.
In the future we may want to introduce other controls for how a
queryId is generated.

Regards,

Sami

---

From: Sami Imseih <[email-masked]>
Date: Tue, 11 Feb 2025 12:00:27 -0600

I do not have an explanation from the patch yet, but I have a test
that appears to show unexpected results. I only tested a few datatypes,
but from what I observe, some merge as expected and others do not;
i.e. int columns merge correctly but bigint do not.

"""
show pg_stat_statements.query_id_const_merge ;
create table foo (col_int int, col_smallint smallint, col_bigint
bigint, col_float float, col_text text, col_varchar varchar);
select from foo where col_int in (1, 2, 3);
select from foo where col_int in (1, 2, 3, 4);
select from foo where col_int in (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
select from foo where col_smallint in (1, 2, 3);
select from foo where col_smallint in (1, 2, 3, 4);
select from foo where col_bigint in (1, 2, 3);
select from foo where col_bigint in (1, 2, 3, 4);
select from foo where col_bigint in (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
select from foo where col_float in (1, 2, 3);
select from foo where col_float in (1, 2, 3, 4);
select query, queryid, calls from pg_stat_statements where query like
'select from foo where%' order by 1 desc ;
"""

postgres=# show pg_stat_statements.query_id_const_merge ;
 pg_stat_statements.query_id_const_merge
-----------------------------------------
 on
(1 row)

....
.......
..........

postgres=# select query, queryid, calls from pg_stat_statements where
query like 'select from foo where%' order by 1 desc ;
                                     query
        |       queryid        | calls
-------------------------------------------------------------------------------+----------------------+-------
 select from foo where col_smallint in (...)
        | -2065640271713949220 |     2
 select from foo where col_int in (...)
        |  2911888824129257715 |     3
 select from foo where col_float in ($1, $2, $3, $4)
        | -6847088148705359339 |     1
 select from foo where col_float in ($1, $2, $3)
        |  1631437678183488606 |     1
 select from foo where col_bigint in ($1, $2, $3, $4, $5, $6, $7, $8,
$9, $10) |  3174053975478689499 |     1
 select from foo where col_bigint in ($1, $2, $3, $4)
        | -5236067031911646410 |     1
 select from foo where col_bigint in ($1, $2, $3)
        | -5529594240898645457 |     1
(7 rows)

---

Sami

---

From: Álvaro Herrera <[email-masked]>
Date: Tue, 11 Feb 2025 19:52:33 +0100

> query_id_const_merge is a pg_stat_statements GUC

I was wondering the same thing and found the explanation 
here:
https://postgr.es/m/[email-masked]

> behavior without needing to enable pg_stat_statements.

I agree.  In fact, pg_stat_activity will behave differently (using
merged query_ids) if this value is turned on, for which you need the
contrib module.  This makes no sense to me.

Besides, the patch cheats in this regard: what Dmitry did was
create a function SetQueryIdConstMerge() which the extension with the
GUC can call to set the value of the variable.  I really don't see that
this is better.  I think we should put the GUC back where it was in v15
of the patch.  (I didn't check what other changes there were
afterwards.)

About the GUC name -- query_id_const_merge -- I think this is too much a
hacker's name.  How about
query_id_merge_values
query_id_merge_value_lists
query_id_squash_constant_lists

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"No es bueno caminar con un hombre muerto"