Re: [PATCH] Optionally record Plan IDs to track plan changes for a query

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Sami Imseih <samimseih@gmail.com>
Cc: Lukas Fittl <lukas@fittl.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Marko M <marko@pganalyze.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Julien Rouhaud <rjuju123@gmail.com>
Date: 2025-02-12T00:08:00Z
Lists: pgsql-hackers
On Mon, Feb 10, 2025 at 02:02:10PM -0600, Sami Imseih wrote:
> I am OK with moving away from "jumble" in-lieu of something else, but
> my thoughts are we should actually call this process "fingerprint"
> ( a term we already use in the queryjumblefuncs.c comment ).
> A fingerprint consists of all the interesting parts of a node tree that are
> appended and the final product is a hash of this fingerprint ( i.e. queryId )
> For node attributes we can specify "fingerprint_ignore"
> or "no_fingerprint". What do you think?

I think that I have a long history of showing a bad naming sense, that
I've done some follow-up API renames even on stable branches because
folks didn't like some names, and that I have a reputation for that on
these lists.  :D

Wikipedia seems to agree with you that "fingerprint" would fit for
this purpose, though:
https://en.wikipedia.org/wiki/Fingerprint_(computing)

Has anybody any comments about that?  That would be a large renaming,
but in the long term is makes sense if we want to apply that to more
than just parse nodes and query strings.  If you do that, it impacts
the file names and the properties, that are hidden in the backend for
most of it, except the entry API and JumbleState.  This last part
impacts some extensions and I have been maintaining one a bit
(pg_hint_plan).

>> The concept of location does not apply to plans, based on the
>> current proposal, so perhaps we should talk about "query normalization
>> location"?
> 
> Are you referring to JUMBLE_LOCATION? and whether to keep it in
> queryjumblefuncs.c ( or jumblefuncs.c as is being proposed )?

Yes, I am referring to the existing jumble location.  I don't quite
see how it fits with the plan part because we don't really have
locations to track.

Point worth noting, Alvaro has mentioned that he was planning to look
at the pgss patch with IN clauses:
https://www.postgresql.org/message-id/202502111214.fcfodex6t3sy@alvherre.pgsql

Adding him in CC here for awareness, but I can see that both of you
are involved on the other thread, as well.  Also adding Julien in CC,
as he has some out-of-core extension code that depends on the jumbling
structures if I recall correctly.
--
Michael

Commits

  1. doc: Update header file mention for CompareType

  2. Allow plugins to set a 64-bit plan identifier in PlannedStmt

  3. Fix indentation of comment in plannodes.h

  4. Reformat node comments in plannodes.h

  5. injection_points: Add routine able to drop all stats

  6. Add pgstat_drop_matching_entries() to pgstats

  7. Move pg_stat_statements query jumbling to core.