MIME-Version: 1.0
References: <CAP53Pkyow59ajFMHGpmb1BK9WHDypaWtUsS_5DoYUEfsa_Hktg@mail.gmail.com>
 <Z5A__Yuy23aXqUad@paquier.xyz> <CAA5RZ0sUPPOpkRZD=Za83op2ngcPC7dp249vcHA-X5YS7p3n8Q@mail.gmail.com>
 <CAP53PkwuFbo3NkwZgxwNRMjMfqPEqidD-SggaoQ4ijotBVLJAA@mail.gmail.com>
 <Z5cDQJM9nnoXuStO@paquier.xyz> <Z5xTb5iBHVGns35R@paquier.xyz>
 <CAP53Pkw1QJHH9UDjkArS=XdAxtK7XWMpZLGHnVMDmhRTp_HYYw@mail.gmail.com> <Z5xgmgyV-wnzMpkO@paquier.xyz>
In-Reply-To: <Z5xgmgyV-wnzMpkO@paquier.xyz>
From: Sami Imseih <samimseih@gmail.com>
Date: Tue, 4 Feb 2025 17:14:48 -0600
Message-ID: <CAA5RZ0vM9AsEqvKued2drKZJ1opt3wbYaDbxGzi-khkNzwn7og@mail.gmail.com>
Subject: Re: [PATCH] Optionally record Plan IDs to track plan changes for a query
To: Michael Paquier <michael@paquier.xyz>
Cc: Lukas Fittl <lukas@fittl.com>, 
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Marko M <marko@pganalyze.com>
Content-Type: text/plain; charset="UTF-8"
Archived-At: <https://www.postgresql.org/message-id/CAA5RZ0vM9AsEqvKued2drKZJ1opt3wbYaDbxGzi-khkNzwn7og%40mail.gmail.com>
Precedence: bulk

>> Separately I've been thinking how we could best have a discussion/review on
>> whether the jumbling of specific plan struct fields is correct. I was
>> thinking maybe a quick wiki page could be helpful, noting why to jumble/not
>> jumble certain fields?

> Makes sense.  This is a complicated topic.

+1 for the Wiki page

I started looking at the set of patches and started with v3-0001.
For that one, I think we need to refactor a bit more for
maintainability/readability.

queryjumblefuncs.c now has dual purposes which is the generic node jumbling
code and now it also has the specific query jumbling code. That seems wrong
from a readability/maintainability perspective.

Here are my high-level thoughts on this:
1. rename queryjumblefuncs.c to jumblefuncs.c
2. move the query jumbling related code to parser/analyze.c,
since query jumbling occurs there during parsing.
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.

I can work on this if you agree.

Regards,

Sami