Обсуждение: display hot standby state in psql prompt

Hi,

Some weeks ago we briefly discussed in the discord channel the
possibility of introducing a psql prompt display option to identify if
the connected database is in hot standby mode, which can be useful when
using multiple hosts in the connection string. Right now, it's using the
in_hot_standby value in prompt.c to determine the database state:

case 'i':
    if (pset.db && PQparameterStatus(pset.db, "in_hot_standby"))
    {
        const char *hs = PQparameterStatus(pset.db, "in_hot_standby");
        if (hs && strcmp(hs, "on") == 0)
            strlcpy(buf, "standby", sizeof(buf));
        else
            strlcpy(buf, "primary", sizeof(buf));

.. which could be used like this:

psql (18beta1)
Type "help" for help.

postgres=# \set PROMPT1 '[%i] # '
[standby] # SELECT pg_promote();
 pg_promote
------------
 t
(1 row)

[primary] #


The hardcoded "standby" and "primary" strings are not very flexible, but
I am not sure how to make these strings customisable just yet.

Any thoughts on this feature?

Best regards, Jim

Hi Greg

On 25.06.25 17:17, Greg Sabino Mullane wrote:
> Seems transaction_read_only might be a more useful thing to examine?
> That's the side-effect, if you will, that people really care about
> when in hot standby mode (and of course, we can get into TRO other ways).

Good point. But wouldn't it mean that I would need to execute a query
every time the prompt is refreshed? Since I cannot get the value of
transaction_read_only via PQparameterStatus. I like the idea, but I'm
concerned about the overhead.

Best, Jim

Hi

On 21.07.25 21:12, Greg Sabino Mullane wrote:
> Seems good enough for me. I think as long as we document it well, it's
> only going to be a net positive, even with some edge cases.

I just moved the patch from PG19-Drafts to PG19-2 commitfest.[1]

Thanks a lot for the feedback!

Best regards, Jim

1 - https://commitfest.postgresql.org/patch/5872/

Hi Srinath

On 23.07.25 09:03, Srinath Reddy Sadipiralla wrote:
> +1 for the patch,i have reviewed and tested this patch, except these
> below cosmetic changes it LGTM.
>
> cosmetic changes:
> 1) add comment about %i in get_prompt api.

Done.

> 2) maybe we can use read-write instead of read/write to be consistent
> with the
>     naming such as options of target_session_attrs uses read-write.

I believe that 'read/write' is more idiomatic than 'read-write' in this
context. 'Read-only' works as a hyphenated adjective, and 'read/write'
is typically treated as a paired label that indicates two distinct
capabilities --- read and write. What do you think?

v3 attached.

Thanks for the thorough testing and review!

Best, Jim

On Wed, Jul 23, 2025 at 10:03:54AM +0200, Jim Jones wrote:
> I believe that 'read/write' is more idiomatic than 'read-write' in this
> context. 'Read-only' works as a hyphenated adjective, and 'read/write'
> is typically treated as a paired label that indicates two distinct
> capabilities --- read and write. What do you think?

My first thought when looking at this thread/patch was that something like
"read-write" might be a bit long for a psql prompt.  I probably would've
chosen something like "r" or "ro" for read-only and "rw" for read-write.  I
suppose you could argue that those are more difficult to see, and this
seems particularly useful when you want to be sure that you won't
inadvertently write any data.

-- 
nathan

Hi Nathan

On 23/10/2025 23:02, Nathan Bossart wrote:
> My first thought when looking at this thread/patch was that something like
> "read-write" might be a bit long for a psql prompt.  I probably would've
> chosen something like "r" or "ro" for read-only and "rw" for read-write.  I
> suppose you could argue that those are more difficult to see, and this
> seems particularly useful when you want to be sure that you won't
> inadvertently write any data.

I can see the appeal of keeping the indicator as short as possible in a
prompt, but that can also make it a bit harder to quickly distinguish
between the read/write and read-only states. In this case, clarity might
be more important than compactness. Then again, the traditional shell
prompt symbols ($, #) aren't exactly self-explanatory either, and people
seem fine with those :)

Although I prefer it being explicit in this case, I can live with either
approach. Do you think we should go for "ro" or "rw"?

Thanks for the review!

Best, Jim

On Fri, Oct 24, 2025 at 01:07:41AM +0200, Jim Jones wrote:
> Although I prefer it being explicit in this case, I can live with either
> approach. Do you think we should go for "ro" or "rw"?

I'm currently leaning towards ro/rw, but I'm also hoping that others chime
in with opinions here.

-- 
nathan

On Fri, Oct 24, 2025 at 10:54:57AM -0400, Greg Sabino Mullane wrote:
> On Fri, Oct 24, 2025 at 10:12 AM Nathan Bossart <nathandbossart@gmail.com>
> wrote:
>> I'm currently leaning towards ro/rw, but I'm also hoping that others chime
>> in with opinions here.
> 
> -1. Too short, too cryptic, too similar. I know our existing symbols aren't
> great either, but we can do better.

What would you prefer instead?

-- 
nathan

On Wed, Jul 23, 2025 at 5:04 PM Jim Jones <jim.jones@uni-muenster.de> wrote:
> v3 attached.

+ const char *hs = PQparameterStatus(pset.db, "in_hot_standby");
+ const char *ro = PQparameterStatus(pset.db, "default_transaction_read_only");

When either hs or ro is NULL, the displayed status can be incorrect.
For example, connecting to a standby server running PostgreSQL 10
incorrectly shows "read/write". In such cases, wouldn't it be clearer
to display something like "unknown", similar to how the "Hot Standby"
column in \conninfo reports "unknown"?

Regards,

--
Fujii Masao

On 24/10/2025 17:21, Fujii Masao wrote:
> + const char *hs = PQparameterStatus(pset.db, "in_hot_standby");
> + const char *ro = PQparameterStatus(pset.db, "default_transaction_read_only");
> 
> When either hs or ro is NULL, the displayed status can be incorrect.
> For example, connecting to a standby server running PostgreSQL 10
> incorrectly shows "read/write". In such cases, wouldn't it be clearer
> to display something like "unknown", similar to how the "Hot Standby"
> column in \conninfo reports "unknown"?

Oh, it didn't occur to me to test this edge case. Thanks for the hint!

Would this be what you have in mind?

if (!hs || !ro)
    strlcpy(buf, "unknown", sizeof(buf));
else if ((hs && strcmp(hs, "on") == 0) ||
    (ro && strcmp(ro, "on") == 0))
    strlcpy(buf, "read-only", sizeof(buf));
else
    strlcpy(buf, "read/write", sizeof(buf));


Best, Jim

On 24/10/2025 18:13, Jim Jones wrote:
> 
> On 24/10/2025 17:21, Fujii Masao wrote:
>> + const char *hs = PQparameterStatus(pset.db, "in_hot_standby");
>> + const char *ro = PQparameterStatus(pset.db, "default_transaction_read_only");
>>
>> When either hs or ro is NULL, the displayed status can be incorrect.
>> For example, connecting to a standby server running PostgreSQL 10
>> incorrectly shows "read/write". In such cases, wouldn't it be clearer
>> to display something like "unknown", similar to how the "Hot Standby"
>> column in \conninfo reports "unknown"?
> 
> Oh, it didn't occur to me to test this edge case. Thanks for the hint!
> 
> Would this be what you have in mind?
> 
> if (!hs || !ro)
>     strlcpy(buf, "unknown", sizeof(buf));
> else if ((hs && strcmp(hs, "on") == 0) ||
>     (ro && strcmp(ro, "on") == 0))
>     strlcpy(buf, "read-only", sizeof(buf));
> else
>     strlcpy(buf, "read/write", sizeof(buf));
> 

I just realised I forgot to attach the patch. Sorry about that!
PFA v4.

Best, Jim

On Sat, Oct 25, 2025 at 1:13 AM Jim Jones <jim.jones@uni-muenster.de> wrote:
>
>
>
> On 24/10/2025 17:21, Fujii Masao wrote:
> > + const char *hs = PQparameterStatus(pset.db, "in_hot_standby");
> > + const char *ro = PQparameterStatus(pset.db, "default_transaction_read_only");
> >
> > When either hs or ro is NULL, the displayed status can be incorrect.
> > For example, connecting to a standby server running PostgreSQL 10
> > incorrectly shows "read/write". In such cases, wouldn't it be clearer
> > to display something like "unknown", similar to how the "Hot Standby"
> > column in \conninfo reports "unknown"?
>
> Oh, it didn't occur to me to test this edge case. Thanks for the hint!
>
> Would this be what you have in mind?

Yes, thanks for working on this!


> if (!hs || !ro)
>     strlcpy(buf, "unknown", sizeof(buf));
> else if ((hs && strcmp(hs, "on") == 0) ||
>     (ro && strcmp(ro, "on") == 0))

I think the "hs &&" and "ro &&" checks are no longer needed,
since we've already confirmed they're not NULL at that point.

Also, should "unknown" be marked for translation, as in the \conninfo code?
I'm not sure whether showing a translated string in the psql prompt is
desirable, though.

Regards,

--
Fujii Masao

On 27/10/2025 04:32, Fujii Masao wrote:
> I think the "hs &&" and "ro &&" checks are no longer needed,
> since we've already confirmed they're not NULL at that point.

Right. Checks removed.

> Also, should "unknown" be marked for translation, as in the \conninfo code?
> I'm not sure whether showing a translated string in the psql prompt is
> desirable, though.

Here I am not sure if it's applicable. In this file there are other
strings that are not marked for translation, "abort", "on", "off". I
changed the string to _("unknown") for now, but we can remove it in
further iterations if we agree it's not desirable :)

v5 attached.

Thanks for the review.

Best, Jim

On Fri, Oct 24, 2025 at 08:43:20PM +0530, Srinath Reddy Sadipiralla wrote:
> On Fri, Oct 24, 2025 at 8:36 PM Greg Sabino Mullane <htamfids@gmail.com>
> wrote:
>> read-only
>> read/write
>
> +1

Alright, I seem to have been outvoted, then.

-- 
nathan

On Mon, Oct 27, 2025 at 09:08:17AM +0100, Jim Jones wrote:
> +            Displays the session's read-only status as <literal>read-only</literal>
> +            if the server is in hot standby (<literal>in_hot_standby</literal> is
> +            <literal>on</literal>) or the default transaction mode is read-only
> +            (<literal>default_transaction_read_only</literal> is <literal>on</literal>),
> +            or <literal>read-write</literal> otherwise. Useful for identifying
> +            sessions that cannot perform writes, such as in replication setups.

This was briefly mentioned upthread, but I'm a little concerned that this
doesn't respond to commands like SET TRANSACTION READ ONLY.  I wonder if we
should mark transaction_read_only as GUC_REPORT and use that instead.  FWIW
I see that we marked search_path as GUC_REPORT somewhat recently (see
commit 28a1121).

-- 
nathan

On 27/10/2025 17:23, Nathan Bossart wrote:
> This was briefly mentioned upthread, but I'm a little concerned that this
> doesn't respond to commands like SET TRANSACTION READ ONLY.  I wonder if we
> should mark transaction_read_only as GUC_REPORT and use that instead.  FWIW
> I see that we marked search_path as GUC_REPORT somewhat recently (see
> commit 28a1121).

You're right, it doesn't. I like the idea, but I'm not sure how to
integrate a transaction-scoped variable into this feature. Would that
mean we also need to change the reset mechanism for GUC_REPORT variables
when the transaction ends?

Best, Jim

On Mon, Oct 27, 2025 at 09:17:03PM +0100, Jim Jones wrote:
> On 27/10/2025 17:23, Nathan Bossart wrote:
>> This was briefly mentioned upthread, but I'm a little concerned that this
>> doesn't respond to commands like SET TRANSACTION READ ONLY.  I wonder if we
>> should mark transaction_read_only as GUC_REPORT and use that instead.  FWIW
>> I see that we marked search_path as GUC_REPORT somewhat recently (see
>> commit 28a1121).
> 
> You're right, it doesn't. I like the idea, but I'm not sure how to
> integrate a transaction-scoped variable into this feature. Would that
> mean we also need to change the reset mechanism for GUC_REPORT variables
> when the transaction ends?

Hm.  You're right, that seems to have problems (I'm curious about the use
of stmt->is_local in SetPGVariable() for SET TRANSACTION statements).  I
also see some past discussions in this area [0] [1] [2].

[0] https://postgr.es/m/flat/3a40f835-116d-0f95-aede-d5236337bbf0%402ndquadrant.com
[1] https://postgr.es/m/flat/CA%2BTgmoZsHrHeqh5dYpoH%2BWW5EmT-egMGuyrLTsjKz80WajT4tg%40mail.gmail.com
[2] https://postgr.es/m/flat/CAFj8pRBFU-WzzQhNrwRHn67N0Ug8a9-0-9BOo69PPtcHiBDQMA%40mail.gmail.com

-- 
nathan

On Tue, Oct 28, 2025 at 6:00 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> On Mon, Oct 27, 2025 at 09:17:03PM +0100, Jim Jones wrote:
> > On 27/10/2025 17:23, Nathan Bossart wrote:
> >> This was briefly mentioned upthread, but I'm a little concerned that this
> >> doesn't respond to commands like SET TRANSACTION READ ONLY.  I wonder if we
> >> should mark transaction_read_only as GUC_REPORT and use that instead.  FWIW
> >> I see that we marked search_path as GUC_REPORT somewhat recently (see
> >> commit 28a1121).
> >
> > You're right, it doesn't. I like the idea, but I'm not sure how to
> > integrate a transaction-scoped variable into this feature. Would that
> > mean we also need to change the reset mechanism for GUC_REPORT variables
> > when the transaction ends?

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.

Regards,

--
Fujii Masao

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.

== guc_parameters.dat ==

{ name => 'transaction_read_only', type => 'bool', context =>
'PGC_USERSET', group => 'CLIENT_CONN_STATEMENT',
  short_desc => 'Sets the current transaction\'s read-only status.',
  flags => 'GUC_NO_RESET | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE |
GUC_DISALLOW_IN_FILE | GUC_REPORT',
  variable => 'XactReadOnly',
  boot_val => 'false',
  check_hook => 'check_transaction_read_only',
},

== prompt.c ==

...
const char *hs = PQparameterStatus(pset.db, "in_hot_standby");
const char *ro = PQparameterStatus(pset.db,
"default_transaction_read_only");
const char *tr = PQparameterStatus(pset.db, "transaction_read_only");

if (!hs || !ro || !tr)
    strlcpy(buf, _("unknown"), sizeof(buf));
else if (strcmp(hs, "on") == 0 || strcmp(ro, "on") == 0 || strcmp(tr,
"on") == 0)
    strlcpy(buf, "read-only", sizeof(buf));
else
    strlcpy(buf, "read/write", sizeof(buf));

...

== test ==

psql (19devel)
Type "help" for help.

postgres=# \set PROMPT1 '[%i] # '
[read/write] # BEGIN;
BEGIN
[read/write] # SET transaction_read_only TO on;
SET
[read-only] # END;
COMMIT
[read-only] # SHOW transaction_read_only;
 transaction_read_only
-----------------------
 off
(1 row)

[read-only] #


So I assumed that the reset happens during transaction cleanup, which
might not send parameter status updates.

Am I missing something?

Thanks!

Best, Jim

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)

-- 
nathan

On 28/10/2025 17:42, Nathan Bossart wrote:
> 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)

Thanks! It does solve the current problem (don't mind the debug messages):

postgres=# \set PROMPT1 '[%i] # '
DEBUG: hs='off' ro='off' tr='off'

[read/write] # BEGIN;
BEGIN
DEBUG: hs='off' ro='off' tr='off'

[read/write] # SET transaction_read_only TO on;
SET
DEBUG: hs='off' ro='off' tr='on'

[read-only] # END;
COMMIT
DEBUG: hs='off' ro='off' tr='off'

But in other cases it now fails, because the parameter status of
transaction_read_only isn't being updated:

postgres=# \set PROMPT1 '[%i] # '
DEBUG: hs='on' ro='off' tr='on'

[read-only] # SELECT pg_promote();
 pg_promote
------------
 t
(1 row)

DEBUG: hs='off' ro='off' tr='on'

[read-only] # SHOW transaction_read_only;
 transaction_read_only
-----------------------
 off
(1 row)

DEBUG: hs='off' ro='off' tr='on'


I played a bit with the following hack, which solves the
transaction_read_only issue ...


diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index a82286cc98..5c7748fbba 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -2551,9 +2551,19 @@ ReportChangedGUCOptions(void)
         * transition from false to true.
         */
        if (in_hot_standby_guc && !RecoveryInProgress())
+       {
                SetConfigOption("in_hot_standby", "false",
                                                PGC_INTERNAL,
PGC_S_OVERRIDE);

+               /*
+                * Similarly, when exiting hot standby,
transaction_read_only may need
+                * to be reported. During hot standby, writes were
blocked regardless
+                * of the transaction_read_only setting, so report the
actual state now.
+                */
+               SetConfigOption("transaction_read_only","off",
+                                               PGC_INTERNAL,
PGC_S_OVERRIDE);
+       }
+
        /* Transmit new values of interesting variables */
        slist_foreach_modify(iter, &guc_report_list)
        {
@@ -3349,6 +3359,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)


== tests ==

postgres=# \set PROMPT1 '[%i] # '
DEBUG: hs='on' ro='off' tr='on'

[read-only] # SELECT pg_promote();
 pg_promote
------------
 t
(1 row)

DEBUG: hs='off' ro='off' tr='off'

[read/write] # BEGIN;
BEGIN
DEBUG: hs='off' ro='off' tr='off'

[read/write] # SET TRANSACTION READ ONLY;
SET
DEBUG: hs='off' ro='off' tr='on'

[read-only] # END;
COMMIT
DEBUG: hs='off' ro='off' tr='off'

[read/write] # SET default_transaction_read_only TO on;
SET
DEBUG: hs='off' ro='on' tr='off'

[read-only] # SET default_transaction_read_only TO off;
SET
DEBUG: hs='off' ro='off' tr='off'
[read/write] #

... but I'm still not sure of its impact. I guess I need to go back to
the drawing board on this one.

Thanks!

Best, Jim

On Tue, Oct 28, 2025 at 8:03 PM Jim Jones <jim.jones@uni-muenster.de> 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.
>
> == guc_parameters.dat ==
>
> { name => 'transaction_read_only', type => 'bool', context =>
> 'PGC_USERSET', group => 'CLIENT_CONN_STATEMENT',
>   short_desc => 'Sets the current transaction\'s read-only status.',
>   flags => 'GUC_NO_RESET | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE |
> GUC_DISALLOW_IN_FILE | GUC_REPORT',
>   variable => 'XactReadOnly',
>   boot_val => 'false',
>   check_hook => 'check_transaction_read_only',
> },
>
> == prompt.c ==
>
> ...
> const char *hs = PQparameterStatus(pset.db, "in_hot_standby");
> const char *ro = PQparameterStatus(pset.db,
> "default_transaction_read_only");
> const char *tr = PQparameterStatus(pset.db, "transaction_read_only");
>
> if (!hs || !ro || !tr)
>         strlcpy(buf, _("unknown"), sizeof(buf));
> else if (strcmp(hs, "on") == 0 || strcmp(ro, "on") == 0 || strcmp(tr,
> "on") == 0)
>         strlcpy(buf, "read-only", sizeof(buf));
> else
>         strlcpy(buf, "read/write", sizeof(buf));
>
> ...
>
> == test ==
>
> psql (19devel)
> Type "help" for help.
>
> postgres=# \set PROMPT1 '[%i] # '
> [read/write] # BEGIN;
> BEGIN
> [read/write] # SET transaction_read_only TO on;
> SET
> [read-only] # END;
> COMMIT
> [read-only] # SHOW transaction_read_only;
>  transaction_read_only
> -----------------------
>  off
> (1 row)
>
> [read-only] #
>
>
> So I assumed that the reset happens during transaction cleanup, which
> might not send parameter status updates.
>
> Am I missing something?

No, you're right. I mistakenly thought marking it as GUC_REPORT was enough
since BEGIN READ ONLY + COMMIT/ROLLBACK worked as expected,
but I overlooked the SET transaction_read_only + COMMIT case where
it doesn't. Sorry for the noise.

Regards,

--
Fujii Masao

> On Oct 30, 2025, at 11:42, Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Tue, Oct 28, 2025 at 8:03 PM Jim Jones <jim.jones@uni-muenster.de> wrote:
>>

I did a quick test, and found a problem. I shutdown the server, and “\c” reconnecting failed, but psql still show
“read/write”,which seems wrong: 

```
"read/write"\c
connection to server on socket "/tmp/.s.PGSQL.5432" failed: No such file or directory
    Is the server running locally and accepting connections on that socket?
Previous connection kept
hs=off ro=off
"read/write"
hs=off ro=off
"read/write"
hs=off ro=off
"read/write"
hs=off ro=off
"read/write”
```

Looks like I am too late to vote. Actually, if I had the chance, I would vote “rw/ro”.

And a question:
```
+                        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
+                            strlcpy(buf, "read/write", sizeof(buf));
```

Why wrap “unknown” in "_()” but not “read-only” and “read/write”?

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Hi Chao

On 30/10/2025 08:51, Chao Li wrote:
> I did a quick test, and found a problem. I shutdown the server, and “\c” reconnecting failed, but psql still show
“read/write”,which seems wrong:
 
> 
> "read/write"\c
> connection to server on socket "/tmp/.s.PGSQL.5432" failed: No such file or directory
>     Is the server running locally and accepting connections on that socket?
> Previous connection kept

I can see that a "read/write" prompt in this case sounds misleading, but
I am not entirely sure it is a problem. The message says "Previous
connection kept", which might suggest that the previous parameter values
are cached? See %p:

postgres=# \set PROMPT1 '[%p] # '
[1268754] # \c
connection to server on socket "/tmp/.s.PGSQL.5432" failed: No such file
or directory
        Is the server running locally and accepting connections on that
socket?
Previous connection kept
[1268754] #

The backend pid is still displayed, although the server is no longer
running.
 > Why wrap “unknown” in "_()” but not “read-only” and “read/write”?
It didn't occur to me that read-only and read/write needed translation,
but I guess you're right. I'll add to the next version.

Thanks for the review. Much appreciated!

Best, Jim

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

> On Oct 30, 2025, at 17:20, Jim Jones <jim.jones@uni-muenster.de> wrote:
>
> Hi Chao
>
> On 30/10/2025 08:51, Chao Li wrote:
>> I did a quick test, and found a problem. I shutdown the server, and “\c” reconnecting failed, but psql still show
“read/write”,which seems wrong: 
>>
>> "read/write"\c
>> connection to server on socket "/tmp/.s.PGSQL.5432" failed: No such file or directory
>> Is the server running locally and accepting connections on that socket?
>> Previous connection kept
>
> I can see that a "read/write" prompt in this case sounds misleading, but
> I am not entirely sure it is a problem. The message says "Previous
> connection kept", which might suggest that the previous parameter values
> are cached? See %p:
>
> postgres=# \set PROMPT1 '[%p] # '
> [1268754] # \c
> connection to server on socket "/tmp/.s.PGSQL.5432" failed: No such file
> or directory
>        Is the server running locally and accepting connections on that
> socket?
> Previous connection kept
> [1268754] #
>

Okay, if you consider that is not a problem. How about this?

```
[read/write]select * from test_multi order by category, name;
FATAL:  terminating connection due to administrator command
server closed the connection unexpectedly
    This probably means the server terminated abnormally
    before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
The connection to the server was lost. Attempting reset: Failed.
[]
[]
```

After shutting down the server, I ran a random statement in psql, then the prompt became empty, does it suppose to show
“_unknown”?

Without setting PROMIT1, the prompt will become “!?>”.
```
evantest=# select * from test_multi order by category, name;
FATAL:  terminating connection due to administrator command
server closed the connection unexpectedly
    This probably means the server terminated abnormally
    before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
The connection to the server was lost. Attempting reset: Failed.
!?>
```

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On 30/10/2025 11:28, Chao Li wrote:
> How about this?
> 
> ```
> [read/write]select * from test_multi order by category, name;
> FATAL:  terminating connection due to administrator command
> server closed the connection unexpectedly
>     This probably means the server terminated abnormally
>     before or while processing the request.
> The connection to the server was lost. Attempting reset: Failed.
> The connection to the server was lost. Attempting reset: Failed.
> []
> []
> ```
> 
> After shutting down the server, I ran a random statement in psql, then the prompt became empty, does it suppose to
show“_unknown”?
 

This behaviour is normal -- at least it is how other options behave. See
%p and the newly committed %S:

postgres=# \set PROMPT1 '[%p] # '
[1376487] # \c
connection to server on socket "/tmp/.s.PGSQL.5432" failed: No such file
or directory
        Is the server running locally and accepting connections on that
socket?
Previous connection kept
[1376487] # SELECT 1;
FATAL:  terminating connection due to administrator command
server closed the connection unexpectedly
        This probably means the server terminated abnormally
        before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
The connection to the server was lost. Attempting reset: Failed.
[] # SELECT 1;
You are currently not connected to a database.
[] #

====

psql (19devel)
Type "help" for help.

postgres=# \set PROMPT1 '[%S] # '
["$user", public] # \c
connection to server on socket "/tmp/.s.PGSQL.5432" failed: No such file
or directory
        Is the server running locally and accepting connections on that
socket?
Previous connection kept
["$user", public] # SELECT 1;
FATAL:  terminating connection due to administrator command
server closed the connection unexpectedly
        This probably means the server terminated abnormally
        before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
The connection to the server was lost. Attempting reset: Failed.
[] # SELECT 1;
You are currently not connected to a database.
[] #


Best, Jim

> On Oct 30, 2025, at 18:50, Jim Jones <jim.jones@uni-muenster.de> wrote:
>
>
>
> On 30/10/2025 11:28, Chao Li wrote:
>> How about this?
>>
>> ```
>> [read/write]select * from test_multi order by category, name;
>> FATAL:  terminating connection due to administrator command
>> server closed the connection unexpectedly
>> This probably means the server terminated abnormally
>> before or while processing the request.
>> The connection to the server was lost. Attempting reset: Failed.
>> The connection to the server was lost. Attempting reset: Failed.
>> []
>> []
>> ```
>>
>> After shutting down the server, I ran a random statement in psql, then the prompt became empty, does it suppose to
show“_unknown”? 
>
> This behaviour is normal -- at least it is how other options behave. See
> %p and the newly committed %S:
>
> postgres=# \set PROMPT1 '[%p] # '
> [1376487] # \c
> connection to server on socket "/tmp/.s.PGSQL.5432" failed: No such file
> or directory
>        Is the server running locally and accepting connections on that
> socket?
> Previous connection kept
> [1376487] # SELECT 1;
> FATAL:  terminating connection due to administrator command
> server closed the connection unexpectedly
>        This probably means the server terminated abnormally
>        before or while processing the request.
> The connection to the server was lost. Attempting reset: Failed.
> The connection to the server was lost. Attempting reset: Failed.
> [] # SELECT 1;
> You are currently not connected to a database.
> [] #
>
> ====
>
> psql (19devel)
> Type "help" for help.
>
> postgres=# \set PROMPT1 '[%S] # '
> ["$user", public] # \c
> connection to server on socket "/tmp/.s.PGSQL.5432" failed: No such file
> or directory
>        Is the server running locally and accepting connections on that
> socket?
> Previous connection kept
> ["$user", public] # SELECT 1;
> FATAL:  terminating connection due to administrator command
> server closed the connection unexpectedly
>        This probably means the server terminated abnormally
>        before or while processing the request.
> The connection to the server was lost. Attempting reset: Failed.
> The connection to the server was lost. Attempting reset: Failed.
> [] # SELECT 1;
> You are currently not connected to a database.
> [] #
>

Thanks for the clarification. Though I still think that might be a bug, but I agree that doesn’t belong to the current
patch.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On 30/10/2025 11:16, Jim Jones wrote:
> 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);
> }

While reviewing another patch, I had another idea to further minimise
the overhead of checking transaction_read_only, namely, to check it only
when within a transaction block:

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
{
  PGTransactionStatusType tstatus = PQtransactionStatus(pset.db);

  /*
   * Check transaction_read_only only when in a transaction
   * block.  When idle (not in a transaction), the value of
   * transaction_read_only is the same as
   * default_transaction_read_only, which we already checked
   * above.  Avoiding the query improves performance,
   * especially for prompt redisplays.
   */

  if (tstatus == PQTRANS_IDLE)
    strlcpy(buf, _("read/write"), sizeof(buf));
  else
  {
    /* In a transaction block, need to check transaction_read_only */
    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);
  }
}

The idea is to skip the test if tstatus == PQTRANS_IDLE, which indicates
that it is not in a transaction block, making the check for
transaction_read_only unnecessary.

Thoughts on this approach?

v7 attached.

Best, Jim