On 02/07/2026 19:30, Bryan Green wrote:
None of this is broken; it works. It's threads pretending to be
processes because the code was written for processes, and the port kept
the protocol rather than rethinking it. I'd like to stop.
One model everywhere should be threads on all platforms, coordinated by
an in-process work queue-- a mutex and a couple of condition variables--
instead of two worker models bridged by an inter-process protocol.
+1
To be clear, the unification is on the queue, not on what Windows does
today. Teaching the non-Windows side to talk to its own threads over a
socket, a byte at a time, would just be the same trick on more
platforms-- that's the part worth deleting, not copying.
I've done the Windows half, both to prove it out and because it's the
coordination layer the non-Windows side would adopt. ...
I see why you developed this that way, and it makes a lot of sense.
However, it has one downside: the Windows-only code cannot be tested
without Windows. At quick glance, it looks reasonable, but I'm a little
nervous committing more Windows-only code without being able to easily
play with it myself.
Once you have the final patch ready to switch non-Windows systems to
threaded model too, that gets easier.
With the thread rework, fmtId's static return value, is now
_Thread_local.
+1. This is the first _Thread_local in our codebase. It's in C11, so it
should just work, but we'll see if the buildfarm shows any surprises...
With this, the getLocalPQExpBuffer hook is never set. I think we can
just remove it, and rename defaultGetLocalPQExpBuffer() to
getLocalPQExpBuffer() directly.
I noticed that we currently call setFmtEncoding() in multiple places in
src/bin/pg_dump. I haven't looked at them closely, but I wonder if
there's some kind of thread-safety hazards there even without these patches.
And what about the 'quote_all_identifiers' global variable? Is that set
correctly in both models? I guess it's inherited through fork().
- Heikki
