MIME-Version: 1.0
References: 
 <CAA5RZ0ui6JSDPQDs2RW7eBDvrH_t_xg8bhUM8FaxawU8wDoMFg@mail.gmail.com>
 <8437F4D0-9DFB-4045-9318-CC3C5BA2E267@paquier.xyz>
 <CAP53Pkwb2f288Ji2s9ZZTL0_PEyYCZpQjBtD63X_s+yUB3YORA@mail.gmail.com>
In-Reply-To: 
 <CAP53Pkwb2f288Ji2s9ZZTL0_PEyYCZpQjBtD63X_s+yUB3YORA@mail.gmail.com>
From: Sami Imseih <samimseih@gmail.com>
Date: Mon, 30 Mar 2026 14:20:47 -0500
Message-ID: 
 <CAA5RZ0vrXNndS6rGx43q+h9JNtxnUzZnxgP_zWSc9LHXS36bXg@mail.gmail.com>
Subject: Re: Refactor query normalization into core query jumbling
To: Lukas Fittl <lukas@fittl.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/CAA5RZ0vrXNndS6rGx43q%2Bh9JNtxnUzZnxgP_zWSc9LHXS36bXg%40mail.gmail.com>
Precedence: bulk

> > > I agree that ComputeConstantLengths should be in core. This one is
> > > not a gray area IMO. The query jumble already records constant locati=
ons,
> > > but leaves the lengths unset. ComputeConstantLengths is just the
> > > completion of that  work. There could be no other interpretation,
> > > unlike generate_normalized_query, of what the lengths should be.
> >
> > This is an interesting remark. One problem with any patches presented y=
et is that we don=E2=80=99t attach the lengths as part of the in-core query=
 jumbling procedure: we plug the lengths later using the same structure as =
the query jumbling. It seems to me that this is half-baked overall. I think=
 that we don=E2=80=99t want to pay the extra length computation in the core=
 query jumbling at the end, then why does it make sense to include the leng=
ths in the JumbleState structure at all? Shouldn=E2=80=99t we use a differe=
nt structure filled inside PGSS for this purpose rather than reuse the same=
 thing for PGSS and the in-core part (don=E2=80=99t have the code in front =
of me now).
>
> I see where you're coming from on that, but I don't think we can
> remove anything here in practice:

Yes. Not unless we want to rely on the parser to track the lengths in
Const, which could be invasive, but I have not looked into it.

Paying the length computation at the end of every jumble is also
going to impact performance, so that will not be a good idea.

> I still think it'd be reasonable for us to include
> ComputeConstantLengths in core to complete the picture of what we're
> doing with _jumbleElements and the length field already anyway. Its
> basically a way to fully hydrate the partially filled out JumbleState
> from the initial jumble.

I fully agree, ComputeConstantLengths is an optional post-jumble-query step
for a consumer that wishes to calculate the lengths. The length calculation
is not unique to a plug-in, so in my mind the work it's doing is core
jumbling functionality.

--
Sami Imseih
Amazon Web Services (AWS)