From: Tom Lane <tgl@sss.pgh.pa.us>
To: Greg Nancarrow <gregn4422@gmail.com>
cc: Alvaro Herrera <alvherre@alvh.no-ip.org>,
        Anastasia Lubennikova <a.lubennikova@postgrespro.ru>,
        Peter Smith <smithpb2250@gmail.com>,
        "Smith,
    Peter" <peters@fast.au.fujitsu.com>,
        David Steele <david@pgmasters.net>,
        "Tsunakawa,
    Takayuki" <tsunakawa.takay@fujitsu.com>,
        Haribabu Kommi <kommi.haribabu@gmail.com>,
        Robert Haas <robertmhaas@gmail.com>,
        Laurenz Albe <laurenz.albe@cybertec.at>,
        Dave Cramer <pg@fastcrypt.com>, Jing Wang <jingwangian@gmail.com>,
        PostgreSQL-development <pgsql-hackers@postgresql.org>,
        Elvis Pranskevichus <elprans@gmail.com>
Subject: Re: Libpq support to connect to standby server as priority
In-reply-to: 
 <CAJcOf-e-H2VTo_0vuf69Hsh2FS4CxxWzkgDohXDGMr8cCMmurw@mail.gmail.com>
References: <20201124225917.GA21884@alvherre.pgsql>
 <325979.1606259987@sss.pgh.pa.us> <388250.1606266434@sss.pgh.pa.us>
 <CAJcOf-cgj=bGOZR7F=H+Rg7846ZgC1G+xbkKKLp6zEuOnZeSFQ@mail.gmail.com>
 <493358.1606322607@sss.pgh.pa.us>
 <CAJcOf-emaiiPOgmpxQyAQT=DAOUPPwZ_G7Z70OdD6ZMX2ifCpw@mail.gmail.com>
 <CAJcOf-e-H2VTo_0vuf69Hsh2FS4CxxWzkgDohXDGMr8cCMmurw@mail.gmail.com>
Comments: In-reply-to Greg Nancarrow <gregn4422@gmail.com>
	message dated "Tue, 01 Dec 2020 19:09:10 +1100"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3425295.1609882492.1@sss.pgh.pa.us>
Date: Tue, 05 Jan 2021 16:34:52 -0500
Message-ID: <3425296.1609882492@sss.pgh.pa.us>
Precedence: bulk

Greg Nancarrow <gregn4422@gmail.com> writes:
> Posting an updated set of patches.

I've reviewed and pushed most of v20-0001, with the following changes:

* I realized that we had more moving parts than necessary for
in_hot_standby.  We don't really need two static variables, one is
sufficient --- and we shouldn't make the SHOW hook have side-effects,
that's just dangerous.

* The documentation patches were missing an addition to config.sgml,
as well as failing to list the new GUC in the two places where we
document all GUC_REPORT variables.

What I did *not* push was the change to mark transaction_read_only
as GUC_REPORT.  I find that idea highly dubious, for a couple of
reasons:

* It'll create useless ParameterStatus traffic during normal operations
of an application using "START TRANSACTION READ ONLY" or the like.

* I do not think it will actually work for the desired purpose of
finding out the read-only state during session connection.  AFAICS,
we don't set XactReadOnly to a meaningful value except when starting
a transaction.  Yeah, we'll run a transaction during login because
we have to examine the system catalogs ... but do we start a new
one after absorbing the effects of, say, ALTER USER SET
default_transaction_read_only?  I doubt it, and even if it works
today it'd be fragile, because someday somebody will try to collapse
any multiple transactions during startup into one transaction.

I think what we want to do is mark default_transaction_read_only as
GUC_REPORT, instead.  That will give a reliable report of what the
state of its GUC is, and you can combine it with is_hot_standby
to decide whether the session should be considered read-only.
If you don't get those two GUC values during connection, then you
can fall back on "SHOW transaction_read_only".

Setting this back to waiting on author.

			regards, tom lane