Re: pg_stat_statements and "IN" conditions

Hi,

Thanks for the updated patch!

I spent some time looking at v24 today, and I have some findings/comments.

1/
Constants passed as parameters to a prepared statement will not be
handled as expected. I did not not test explicit PREPARE/EXECUTE statement,
but I assume it will have the same issue.

postgres=# show query_id_squash_values;
query_id_squash_values
------------------------
on
(1 row)

postgres=# select from foo where col_bigint in ($1, $2, $3, $4) \bind 1 2 3 4
postgres-# ;
--
(0 rows)

postgres=# select from foo where col_bigint in ($1, $2, $3, $4, $5)
\bind 1 2 3 4 5
postgres-# ;
--
(0 rows)

postgres=# select query, queryid, calls from pg_stat_statements where
query like 'select%from foo where%' order by stats_since asc;
query |
queryid | calls
----------------------------------------------------------+----------------------+-------
select from foo where col_bigint in ($1, $2, $3, $4) |
-1169585827903667511 | 1
select from foo where col_bigint in ($1, $2, $3, $4, $5) |
-5591703027615838766 | 1
(2 rows)

I think the answer is here is to also check for "Param" when deciding
if an element
should be merged.

i.e.

if (!IsA(element, Const) && !IsA(element, Param))

2/

This case with an array passed to aa function seems to cause a regression
in pg_stat_statements query text. As you can see the text is incomplete.

CREATE OR REPLACE FUNCTION arrtest(i int[]) RETURNS void AS $$
BEGIN
NULL;
END;
$$ LANGUAGE plpgsql;

postgres=# select arrtest(array[1, 2]) from foo where col_bigint in (1, 2, 3);
arrtest
---------
(0 rows)

postgres=# select query from pg_stat_statements;
query
---------------------------
select arrtest(array[...)
(1 row)

it should otherwise look like this:

postgres=# select query from pg_stat_statements;
query
-------------------------------------------------------------------------
select arrtest(array[$1, $2]) from foo where col_bigint in ($3, $4, $5)
(1 row)

3/

A typo in the docs.

c/lenght/length

+ occurrence with an array of different lenght.

4/

+ <para>
+ Specifies how an array of constants (e.g. for an <literal>IN</literal>
+ clause) contributes to the query identifier computation.

Is this parameter specific to only useful to merge the values of an IN list.
Should the documentation be more specific and say that only IN lists
will benefit from this parameter?

Also, if there is only 1 value in the list, it will have a different
queryId than
that of the same query in which more than 1 value is passed to the IN list.
Should the documentation be clear about that?

5/

pg_node_attr of query_jumble_merge is doing something
very specific to the elements list of an ArrayExpr. The
merge code likely cannot be used for other node types.

/* the array elements or sub-arrays */
- List *elements;
+ List *elements pg_node_attr(query_jumble_merge);

Why are we creating a new node attribute rather than following the
existing pattern of using the "custom_query_jumble" attribute on
ArrayExpr and creating a custom jumble function like we do with
_jumbleVariableSetStmt?

Regards,

Sami