Thanks for detailed explanation. I will start with a high level reorganizing and refactoring plan and a diagram illustrating what I want to achieve.
And maybe even before that a more detailed description of the current state to make the plan and the end goals easier to to discuss. And until we have the ability in commitfest to link patches I will provide instructions on how to get the actual pghackers-style patches from my github if it gets to that point. ------------ Hannu On Sat, Oct 24, 2026 at 6:36 AM Zsolt Parragi <[email protected]> wrote: > > On Fri, 23 Oct 2026, Koshi Shibagaki (Fujitsu) > <[email protected]> wrote: > > For v6, patches 0001-0007 seem like a suitable scope for that review. > > I have left 0008-0012 out of this scope because they go beyond code > > movement and include changes to internal interfaces and structures. > > Those changes could be discussed separately, along with the new > > features. > > +1 > > As I promised at pgconfeu, I tried to take another look at 1-7 later > to see if they are properly code-movement commits. > > But generally I would suggest the same (and what was also suggested in > the other thread[1]): put the idea, "overall PoC" part on github or > another git hosting, resending the the entire patchset every time > doesn't help. > > A simple link and summary would be enough in this thread for anybody > interested in it to take a look at it. > > I am sure people could argue about just the first 7 patches, where to > put which function and such, and constantly rebasing and resending 27 > patches doesn't seem to be that useful to me. It also seems unlikely > for somebody to commit this in one go, so from this perspective, > keeping a single commitfest entry for everything is also wrong. > > That could also help you to focus on the quality of the first patches. > > One thing I immediately noticed is that the summary of 0004 is strange: > > src/bin/pgbench/Makefile | 1 + > src/bin/pgbench/meson.build | 1 + > src/bin/pgbench/pgbench.c | 6419 +++++++++++------------------------ > src/bin/pgbench/pgbench.h | 253 +- > src/bin/pgbench/script.c | 1946 +++++++++++ > src/bin/pgbench/script.h | 170 + > 6 files changed, 4434 insertions(+), 4356 deletions(-) > > If we are only moving code out from pgbench.c, why do we see thousands > of added lines to it? So far I only took a quick glance at it, but > seems like it still mixes things together: internal reorganization > (such as commandFailed is now at a different place in the file), and > moving code out of it. > > The reorganization part isn't mentioned anywhere in the commit message: > > Move the script/command data structures (MetaCommand, QueryMode, Command, > ParsedScript, BuiltinScript), PRNG/distribution/hashing/permutation > functions, expression evaluator, script selector, and SQL/backslash > script parser out of pgbench.c into script.c and script.h. Also move the > shared client/thread state types (ConnectionStateEnum, CState, TStatus, > TState) into pgbench.h so both pgbench.c and script.c can access them. > > I would probably argue that this part of the really long commit > message is already two things and should be two or more separate > patches to reduce the size (script.c itself is 2000 lines), and it > clearly doesn't mention "reorganizing pgbench.c" anywhere. > > And it shouldn't, I still think one patchfile should focus on a single > well defined change, that can be described in a few sentences. > > Alternatively, since we are talking about simple reorganization, maybe > it shouldn't even start with patches? Maybe a table (CSV?) of what > functions to move where, and/or some text based diagram format like > mermaid to visualize the architecture/structure you want to get, if > you want to talk about the long term vision? > > A properly written pitch/design document could be an easier way for > people to argue about it, and then it could help solve the question of > how something this large can get committed? > > [1]: https://www.postgresql.org/message-id/atwDmsIb9TYfg9uH%40paquier.xyz
