public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: Sami Imseih <[email protected]>
Cc: Lukas Fittl <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Marko M <[email protected]>
Subject: Re: [PATCH] Optionally record Plan IDs to track plan changes for a query
Date: Wed, 5 Feb 2025 09:12:14 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAA5RZ0vM9AsEqvKued2drKZJ1opt3wbYaDbxGzi-khkNzwn7og@mail.gmail.com>
References: <CAP53Pkyow59ajFMHGpmb1BK9WHDypaWtUsS_5DoYUEfsa_Hktg@mail.gmail.com>
<[email protected]>
<CAA5RZ0sUPPOpkRZD=Za83op2ngcPC7dp249vcHA-X5YS7p3n8Q@mail.gmail.com>
<CAP53PkwuFbo3NkwZgxwNRMjMfqPEqidD-SggaoQ4ijotBVLJAA@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAP53Pkw1QJHH9UDjkArS=XdAxtK7XWMpZLGHnVMDmhRTp_HYYw@mail.gmail.com>
<[email protected]>
<CAA5RZ0vM9AsEqvKued2drKZJ1opt3wbYaDbxGzi-khkNzwn7og@mail.gmail.com>
On Tue, Feb 04, 2025 at 05:14:48PM -0600, Sami Imseih wrote:
> 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.
> 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.
> 3. Rewrite the comments in the new jumblefuncs.c to
> make it clear the intention of this infrastructure; that
> it is used to jumble nodes for query or plan trees.
Seems to me that this could be done before 2, as well.
> I can work on this if you agree.
I'd welcome an extra patch to rework a bit the format of the comments
for the Plan nodes, to ease the addition of pg_node_attr(), making any
proposed patches more readable.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, 2-signature.asc)
download
view thread (34+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [PATCH] Optionally record Plan IDs to track plan changes for a query
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox