MIME-Version: 1.0
References: 
 <CAA5RZ0tZp5qU0ikZEEqJnxvdSNGh1DWv80sb-k4QAUmiMoOp_Q@mail.gmail.com>
 <aUT1eblRYpwB1nC7@ip-10-97-1-34.eu-west-3.compute.internal>
 <CAA5RZ0vQfE14HyfpoPXDRThVcdCkLgY_HGz+J2qLB9soNUE9QQ@mail.gmail.com>
 <tencent_2FF1F5A9360E2DD24C6478FB@qq.com>
 <CAA5RZ0uLS9RrpO2roX7p3EHE4-VJkBsGAB970jrbo1-GRDAi0g@mail.gmail.com>
 <aUtm47pcAhZ1Ix0B@paquier.xyz>
 <CAA5RZ0sbWmqdUBFo8JXMJe72pnwjxVY58htJ6pKbwnyQuRctQw@mail.gmail.com>
 <CAP53PkxqGbPw5VzpacyJb2wTofYJadCoUmxV8s2o5tHzKznwbg@mail.gmail.com>
 <CAA5RZ0t3-srmtaVNfzv0HEzXCa78eQDKYVak+3zgEccnKPZHHA@mail.gmail.com>
 <CAA5RZ0t_CJs9ye1PPBy-e0ajZ-6FgKvNEoacW9keCtF_7Y2ycQ@mail.gmail.com>
 <CAA5RZ0vEnNvp7_Ni8bWwh4GE53rg_YwNjqynQG=JpNu3YzzAWA@mail.gmail.com>
In-Reply-To: 
 <CAA5RZ0vEnNvp7_Ni8bWwh4GE53rg_YwNjqynQG=JpNu3YzzAWA@mail.gmail.com>
From: Lukas Fittl <lukas@fittl.com>
Date: Wed, 25 Mar 2026 18:34:02 -0700
Message-ID: 
 <CAP53PkzCzeDpg6dCzVrU17LCiCmRSWfEt-5dr4y+VHnTxKr_9w@mail.gmail.com>
Subject: Re: Refactor query normalization into core query jumbling
To: Sami Imseih <samimseih@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, zengman <zengman@halodbtech.com>,
	pgsql-hackers <pgsql-hackers@postgresql.org>,
 Julien Rouhaud <rjuju123@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAP53PkzCzeDpg6dCzVrU17LCiCmRSWfEt-5dr4y%2BVHnTxKr_9w%40mail.gmail.com>
Precedence: bulk

Hi Sami,

On Mon, Mar 16, 2026 at 7:12=E2=80=AFPM Sami Imseih <samimseih@gmail.com> w=
rote:
>
> > Here is v4.
> >
> > 0001 - addresses the comments made by Bertrand.
> > 0002 - makes JumbleState a constant when passed to post_parse_analyze
> > and updates all downstream code that consume the JumbleState. This
> > means we now need to copy the locations from JState into a local/tempor=
ary
> > array when generating the normalized string.

In 0001:

> diff --git a/src/backend/nodes/queryjumblefuncs.c b/src/backend/nodes/que=
ryjumblefuncs.c
> > index 87db8dc1a32..d4b26202c47 100644
> --- a/src/backend/nodes/queryjumblefuncs.c
> +++ b/src/backend/nodes/queryjumblefuncs.c
> ...
> @@ -773,3 +775,249 @@ _jumbleRangeTblEntry_eref(JumbleState *jstate,
> +
> +    /* we don't want to re-emit any escape string warnings */
> +    yyextra.escape_string_warning =3D false;
> +

I don't think this is needed anymore, as of
45762084545ec14dbbe66ace1d69d7e89f8978ac.

> +/*
> + * Callback to generate a normalized version of the query string that wi=
ll be used to
> + * represent all similar queries.
> + *

I don't think the term "Callback" makes sense here - I think you could
just keep the original wording.

In 0002:

> diff --git a/src/backend/nodes/queryjumblefuncs.c b/src/backend/nodes/que=
ryjumblefuncs.c
> index d4b26202c47..fe8f0ccd278 100644
> --- a/src/backend/nodes/queryjumblefuncs.c
> +++ b/src/backend/nodes/queryjumblefuncs.c
> ...
> @@ -813,14 +815,20 @@ SetConstantLengths(JumbleState *jstate, const char =
*query,
>     core_YYSTYPE yylval;
>     YYLTYPE        yylloc;
>
> +    if (jstate->clocations_count =3D=3D 0)
> +        return NULL;
> +
> +    /* Copy constant locations to avoid modifying jstate */
> +    locs =3D palloc(jstate->clocations_count * sizeof(LocationLen));
> +    memcpy(locs, jstate->clocations, jstate->clocations_count * sizeof(L=
ocationLen));
> +

You could use palloc_array for locs here.

> @@ -938,12 +948,13 @@ GenerateNormalizedQuery(JumbleState *jstate, const =
char *query,
>                 last_off =3D 0,    /* Offset from start for previous tok =
*/
>                 last_tok_len =3D 0;    /* Length (in bytes) of that tok *=
/
>     int            num_constants_replaced =3D 0;
> +    LocationLen *locs =3D NULL;
>
>     /*
>      * Set constants' lengths in JumbleState, as only locations are set d=
uring
>      * DoJumble(). Note this also ensures the items are sorted by locatio=
n.
>      */
> -    SetConstantLengths(jstate, query, query_loc);
> +    locs =3D SetConstantLengths(jstate, query, query_loc);

I think we should update the comment here to reflect the fact that
we're no longer modifying JumbleState.

Otherwise these patches look good - it'd be nice to still get this
into 19 so we have less code duplication across the different
extensions working with normalized query text.

Thanks,
Lukas

--=20
Lukas Fittl