Re: pg_stat_statements and "IN" conditions

Hi,

In <20240404143514(dot)a26f7ttxrbdfc73a(at)erthalion(dot)local>
"Re: pg_stat_statements and "IN" conditions" on Thu, 4 Apr 2024 16:35:14 +0200,
Dmitry Dolgov <9erthalion6(at)gmail(dot)com> wrote:

> Here is the rebased version.

Thanks. I'm not familiar with this code base but I've
reviewed these patches because I'm interested in this
feature too.

0001:

> diff --git a/src/backend/nodes/queryjumblefuncs.c b/src/backend/nodes/queryjumblefuncs.c
> index be823a7f8fa..e9473def361 100644
> --- a/src/backend/nodes/queryjumblefuncs.c
> +++ b/src/backend/nodes/queryjumblefuncs.c
>
> @@ -212,15 +233,67 @@ RecordConstLocation(JumbleState *jstate, int location)
> ...
> +static bool
> +IsMergeableConstList(List *elements, Const **firstConst, Const **lastConst)
> +{
> + ListCell *temp;
> + Node *firstExpr = NULL;
> +
> + if (elements == NULL)

"elements == NIL" will be better for List.

> +static void
> +_jumbleElements(JumbleState *jstate, List *elements)
> +{
> + Const *first, *last;
> + if(IsMergeableConstList(elements, &first, &last))

A space is missing between "if" and "(".

> diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
> index aa727e722cc..cf4f900d4ed 100644
> --- a/src/include/nodes/primnodes.h
> +++ b/src/include/nodes/primnodes.h
> @@ -1333,7 +1333,7 @@ typedef struct ArrayExpr
> /* common type of array elements */
> Oid element_typeid pg_node_attr(query_jumble_ignore);
> /* the array elements or sub-arrays */
> - List *elements;
> + List *elements pg_node_attr(query_jumble_merge);

Should we also update the pg_node_attr() comment for
query_jumble_merge in nodes.h?

0003:

> diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
> index d7841b51cc9..00eec30feb1 100644
> --- a/contrib/pg_stat_statements/pg_stat_statements.c
> +++ b/contrib/pg_stat_statements/pg_stat_statements.c
> ...
> @@ -2883,12 +2886,22 @@ generate_normalized_query(JumbleState *jstate, const char *query,
> /* The firsts merged constant */
> else if (!skip)
> {
> + static const uint32 powers_of_ten[] = {
> + 1, 10, 100,
> + 1000, 10000, 100000,
> + 1000000, 10000000, 100000000,
> + 1000000000
> + };
> + int lower_merged = powers_of_ten[magnitude - 1];
> + int upper_merged = powers_of_ten[magnitude];

How about adding a reverse function of decimalLength32() to
numutils.h and use it here?

> - n_quer_loc += sprintf(norm_query + n_quer_loc, "...");
> + n_quer_loc += sprintf(norm_query + n_quer_loc, "... [%d-%d entries]",
> + lower_merged, upper_merged - 1);

Do we still have enough space in norm_query for this change?
It seems that norm_query expects up to 10 additional bytes
per jstate->clocations[i].

It seems that we can merge 0001, 0003 and 0004 to one patch.
(Sorry. I haven't read all discussions yet. If we already
discuss this, sorry for this noise.)

Thanks,
--
kou