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