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

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Sami Imseih <samimseih(at)gmail(dot)com>
Cc: Lukas Fittl <lukas(at)fittl(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Marko M <marko(at)pganalyze(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Julien Rouhaud <rjuju123(at)gmail(dot)com>
Subject: Re: [PATCH] Optionally record Plan IDs to track plan changes for a query
Date: 2025-02-12 00:08:00
Message-ID: Z6vmYBgLRLXkx5fi@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2025-02-12 00:50:54 Re: WAL-logging facility for pgstats kinds
Previous Message Andres Freund 2025-02-12 00:03:27 Re: Allow io_combine_limit up to 1MB