Re: Refactor query normalization into core query jumbling

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Lukas Fittl <lukas(at)fittl(dot)com>
Cc: Sami Imseih <samimseih(at)gmail(dot)com>, zengman <zengman(at)halodbtech(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Julien Rouhaud <rjuju123(at)gmail(dot)com>
Subject: Re: Refactor query normalization into core query jumbling
Date: 2026-03-27 05:17:57
Message-ID: acYTBcq5-tck1zjh@paquier.xyz
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Mar 26, 2026 at 06:50:20PM -0700, Lukas Fittl wrote:
> On Thu, Mar 26, 2026 at 10:19 AM Sami Imseih <samimseih(at)gmail(dot)com> wrote:
> > Both of those changes belong in 0002. See the attached v7.
>
> Looks good!
>
> I've also done a quick follow-up test with pg_tracing and that
> simplifies its logic as expected [0] to be able to extract inline
> parameter values.

I have been looking at what you have here. As mentioned upthread, I
am on board with aiming for JumbleState to be a const so as we enforce
as policy that no extension is allowed to touch what the core code has
computed.

However, I am not convinced by most of the patch. The logic to
recompute the lengths of the constants is a concept proper to PGSS.
Perhaps we could reconsider moving that into core at some point, but
I am honestly not sure to see the range of benefits we'd gain from
that.

This line of arguments is stronger for the normalization of the query
string. Why should the core code decide what a normalized string
should look like when it comes to the detection of the constants, if
any? Instead of a dollar-quoted number, we could enforce a bunch of
things, like a '?' or a '$woozah$' at these locations.

Saying that, the key point of the patch is to take a copy of the
JumbleState locations, and recompute it for the needs of PGSS for the
sake of the normalized query representation. Hence, why don't we just
do that at the end? That should be enough to enforce the const marker
for the JumbleState across all the loaded extensions that want to look
at it. This leads me to the simpler patch attached.

Comments or tomatoes? That's simpler, and I'd be OK with just that
for v19. That would be much better than the current state of affairs,
where PGSS decides to enforce its own ideas in the JumbleState, ideas
fed to anything looping into the post-parse-analyze hook after PGSS.
--
Michael

Attachment Content-Type Size
v8-0001-Make-JumbleState-const-in-post_parse_analyze-hook.patch text/plain 9.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Postgress Cybrosys 2026-03-27 05:32:10 [PATCH] pg_prewarm: validate that first_block <= last_block
Previous Message Nisha Moond 2026-03-27 04:57:43 Re: Use SIGTERM instead of SIGUSR1 for slotsync worker to exit during promotion?