Re: Bug in pg_stat_statements

On Mon, Oct 27, 2025 at 02:34:50PM -0500, Sami Imseih wrote:
> The excellent question raised earlier is whether the fix should be
> applied in pg_stat_statements.c or queryjumblefuncs.c. To me, this
> suggests that pg_stat_statements, as an extension, is dealing with code
> it should not be responsible for.
>
> generate_normalized_query could be used by other extensions that have
> normalization needs; it’s generic and needs to handle squashing, which
> is a jumble specific detail.

FWIW, the line defining the separation between pg_stat_statements.c
and queryjumblefuncs.c should rely on a pretty simple concept:
JumbleState can be written only by queryjumblefuncs.c and
src/backend/nodes/, and should remain in an untouched constant state
when looked at by any external module loaded, included PGSS, because
it could be shared across more than one paths.

That's just to say that I can get behind the argument you are
presenting in patch 0002, to move the updates of
JumbleState.clocations[N].length to happen inside the query jumbling
code.  This relies on a query string anyway, which would be the same
for all the modules interested in interacting in a planner, analyze or
execution hook.

Now, I don't think that 0002 goes far enough: could it be possible to
do things so as we enforce a policy where modules cannot touch a
JumbleState at all?  That would mean painting more consts to prevent
manipulations of the Jumble state.

> Therefore, I think both fill_in_constant_lengths and
> generate_normalized_query should be moved. We can certainly start a new
> thread (0002) if more discussion is needed.

Yeah, that should be a separate discussion.  Let's focus on the bug at
hand, first, then consider improvements that could be done moving
forward.
--
Michael