Re: Bug in pg_stat_statements

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Sami Imseih <samimseih(at)gmail(dot)com>
Cc: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Konstantin Knizhnik <knizhnik(at)garret(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Álvaro Herrera <alvherre(at)kurilemu(dot)de>
Subject: Re: Bug in pg_stat_statements
Date: 2025-10-28 03:51:27
Message-ID: aQA9v9nLu5qsX8IE@paquier.xyz
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2025-10-28 04:02:06 Re: Making pg_rewind faster
Previous Message jian he 2025-10-28 03:33:51 Re: Docs and tests for RLS policies applied by command type