Re: pg_stat_statements and "IN" conditions

Hi,

In <20240404143514.a26f7ttxrbdfc73a@erthalion.local>
  "Re: pg_stat_statements and "IN" conditions" on Thu, 4 Apr 2024 16:35:14 +0200,
  Dmitry Dolgov <9erthalion6@gmail.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