Re: [PATCH] Refactor pgbench to make future improvements easier

On Fri, 23 Oct 2026, Koshi Shibagaki (Fujitsu)
<shibagaki(dot)koshi(at)fujitsu(dot)com> 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