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

Sami Imseih <samimseih@gmail.com>

View thread View on postgr.es

From: Sami Imseih <samimseih@gmail.com>

To: Lukas Fittl <lukas@fittl.com>, Michael Paquier <michael@paquier.xyz>

Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Marko M <marko@pganalyze.com>

Date: 2025-02-07T01:52:53Z

Lists: pgsql-hackers

Attachments

v5-0002-Allow-using-jumbling-logic-outside-of-query-jumbl.patch (application/octet-stream) patch v5-0002
v5-0001-reformat-comments-in-plannode.h.patch (application/octet-stream) patch v5-0001
v5-0004-Add-pg_stat_plans-contrib-extension.patch (application/octet-stream) patch v5-0004
v5-0003-Optionally-record-a-plan_id-in-PlannedStmt-to-ide.patch (application/octet-stream) patch v5-0003

> This does not yet move field-specific comments to their own line in nodes where we're adding node attributes, I'll leave that for Sami to work on.
>

Hi,

Attached is a new set of patches for fixing the long comments
in plannodes.h and to refactor queryjumblefuncs.c

v5-0001
-----------

This fixes the long comments in plannodes.h to make it easier to add the
attribute annotation. It made the most sense to make this the first patch
in the set.


v5-0002
-----------
>> Here are my high-level thoughts on this:
>> 1. rename queryjumblefuncs.c to jumblefuncs.c

> If these APIs are used for somethings else than Query structure, yes,
> the renaming makes sense.

Done. Also rewrote the header comment in jumblefuncs.c to describe
a more generic node jumbling mechanism that this file now offers.

>> 2. move the query jumbling related code to parser/analyze.c,
>> since query jumbling occurs there during parsing.

> Not sure about this one.  It depends on how much is changed.  As long
> as everything related to the nodes stays in src/backend/nodes/,
> perhaps that's OK.

Yes, after getting my hands on this, I agree with you. It made more sense
to keep all the jumbling work in jumblefuncs.c

v5-0003 and v5-0004 introduce the planId in core and pg_stat_plans. These
needed rebasing only; but I have not yet looked at this thoroughly.

We should aim to get 0001 and 0002 committed next.

Regards,

Sami

Commits

doc: Update header file mention for CompareType
- b39013b7b1b1 19 (unreleased) cited
Allow plugins to set a 64-bit plan identifier in PlannedStmt
- 2a0cd38da5cc 18.0 landed
Fix indentation of comment in plannodes.h
- c9238ad85325 18.0 landed
Reformat node comments in plannodes.h
- 3d17d7d7fb7a 18.0 landed
injection_points: Add routine able to drop all stats
- a632cd354d35 18.0 landed
Add pgstat_drop_matching_entries() to pgstats
- ce5c620fb625 18.0 landed
Move pg_stat_statements query jumbling to core.
- 5fd9dfa5f50e 14.0 cited