This patch is marked as "ready for committer", but that characterization
seems way over-optimistic. It looks like there are several unsettled
questions:
1. The connection parameter name and values are unlike the very similar
feature in pgJDBC. I think this is a fair complaint. Now I'm not in love
with "hostRecheckSeconds" --- that seems like a very squishily defined
thing with limited use-case, given Robert's argument that you shouldn't
be using this feature at all for short-lived connections. And
"loadBalanceHosts" is something we could leave for later. But it seems
pretty unfortunate not to follow pgJDBC's lead on the main parameter,
"targetServerType=(any | master | secondary | preferSecondary)".
The problem here of course is that whoever invented target_session_attrs
was unconcerned with following that precedent, so what we have is
"target_session_attrs=(any | read-write)".
Are we prepared to add some aliases in service of unifying these names?
2. Whether or not you want to follow pgJDBC's naming, it seems like we
ought to have both "require read only" and "prefer read only" behaviors
in this patch, and maybe likewise "require read write" versus "prefer
read write".
3. We ought to sync this up with whatever's going to happen in
https://commitfest.postgresql.org/21/1090/
at least to the extent of agreeing on what GUCs we'd like to see
the server start reporting.
4. Given that other discussion, it's not quite clear what we should
even be checking. The existing logic devolves to checking that
transaction_read_only is true, but that's not really the same thing as
"is a master server", eg you might have connected to a master server
under a role that has SET ROLE default_transaction_read_only = false.
(I wonder what pgJDBC is really checking, under the hood.)
Do we want to have modes that are checking hot-standby state in some
fashion, rather than the transaction_read_only state?
regards, tom lane
