| 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 |
| 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? |