On 28/10/2025 17:42, Nathan Bossart wrote:
> On Tue, Oct 28, 2025 at 12:03:48PM +0100, Jim Jones wrote:
>> On 28/10/2025 00:55, Fujii Masao wrote:
>>> If we mark transaction_read_only as GUC_REPORT, wouldn't the reset value
>>> be sent automatically at the end of the transaction? It seems like we wouldn't
>>> need any new mechanism for that. However, the downside might be that
>>> more ParameterStatus messages would be sent, potentially adding overhead.
>>
>> I tried that, but simply marking it as GUC_REPORT does not reset the
>> variable when the transaction ends.
>
> IIUC the problem is that we use GUC_ACTION_SET for those even though they
> are reset at transaction end by the routines in xact.c. Something like the
> following seems to be enough to get it working as expected in some basic
> tests, but there are probably other things to consider. Keep in mind that
> previous proposals to mark transaction_read_only as GUC_REPORT have been
> rejected, too.
>
> diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
> index a82286cc98a..d0bbb5aff19 100644
> --- a/src/backend/utils/misc/guc.c
> +++ b/src/backend/utils/misc/guc.c
> @@ -3349,6 +3349,12 @@ set_config_with_handle(const char *name, config_handle *handle,
> bool prohibitValueChange = false;
> bool makeDefault;
>
> + if (action == GUC_ACTION_SET &&
> + (strcmp(name, "transaction_isolation") == 0 ||
> + strcmp(name, "transaction_read_only") == 0 ||
> + strcmp(name, "transaction_deferrable") == 0))
> + action = GUC_ACTION_LOCAL;
> +
> if (elevel == 0)
> {
> if (source == PGC_S_DEFAULT || source == PGC_S_FILE)
>
Considering the potential overhead of marking transaction_read_only as
GUC_REPORT, and the fact that this change has been rejected in the past,
I think simply calling SHOW transaction_read_only when needed would be a
more palatable approach -- at least a less invasive one.
To minimise overhead, the current implementation first checks the
session-level parameters (default_transaction_read_only and
in_hot_standby) via PQparameterStatus(). If both indicate "read/write"
mode, it then queries the server for the transaction-level
transaction_read_only setting. This means no extra queries are issued on
hot standby systems or sessions with default_transaction_read_only = on:
if (!hs || !ro)
strlcpy(buf, _("unknown"), sizeof(buf));
else if (strcmp(hs, "on") == 0 || strcmp(ro, "on") == 0)
strlcpy(buf, _("read-only"), sizeof(buf));
else
{
const char *tr = NULL;
PGresult *res;
res = PQexec(pset.db, "SHOW transaction_read_only");
if (PQresultStatus(res) == PGRES_TUPLES_OK &&
PQntuples(res) == 1)
tr = PQgetvalue(res, 0, 0);
if (!tr)
strlcpy(buf, _("unknown"), sizeof(buf));
else if (strcmp(tr, "on") == 0)
strlcpy(buf, _("read-only"), sizeof(buf));
else
strlcpy(buf, _("read/write"), sizeof(buf));
PQclear(res);
}
As pointed out by Chao Li, I marked "read/write" and "read-only" for
translation.
== test ==
psql (19devel)
Type "help" for help.
postgres=# \set PROMPT1 '[%i] # '
[read-only] # SHOW in_hot_standby;
in_hot_standby
----------------
on
(1 row)
[read-only] # SELECT pg_promote();
pg_promote
------------
t
(1 row)
[read/write] # SET default_transaction_read_only TO on;
SET
[read-only] # SET default_transaction_read_only TO off;
SET
[read/write] # BEGIN;
BEGIN
[read/write] # SET transaction_read_only TO on;
SET
[read-only] # END;
COMMIT
[read/write] #
Any thoughts on this approach?
v6 attached.
Best, Jim
