Date: Sun, 30 May 2021 20:25:00 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Hubert Zhang <zhubert@vmware.com>, tsunakawa.takay@fujitsu.com,
	pgsql-hackers@postgresql.org,
	Andreas Seltenreich <seltenreich@gmx.de>,
	Bruce Momjian <bruce@momjian.us>
Subject: Re: Multiple hosts in connection string failed to failover in
 non-hot standby mode
Message-ID: <20210531012500.GP2082@telsasoft.com>
References: 
 <BN6PR05MB3492948E4FD76C156E747E8BC9160@BN6PR05MB3492.namprd05.prod.outlook.com>
 <TYAPR01MB299012CAC510D2644AEC2246FE160@TYAPR01MB2990.jpnprd01.prod.outlook.com>
 <BN6PR05MB3492DFB2FF32B771172CC2F2C9170@BN6PR05MB3492.namprd05.prod.outlook.com>
 <229507.1610056206@sss.pgh.pa.us>
 <770711.1610318330@sss.pgh.pa.us>
 <20210506162651.GJ27406@telsasoft.com>
 <4170264.1620321747@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4170264.1620321747@sss.pgh.pa.us>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20210531012500.GP2082%40telsasoft.com>
Precedence: bulk

On Thu, May 06, 2021 at 01:22:27PM -0400, Tom Lane wrote:
> Justin Pryzby <pryzby@telsasoft.com> writes:
> > 52a10224 broke sqlsmith, of all things.
> 
> > It was using errmsg as a test of success, instead of checking if the connection
> > result wasn't null:
> 
> >      conn = PQconnectdb(conninfo.c_str());
> >      char *errmsg = PQerrorMessage(conn);
> >      if (strlen(errmsg))
> >          throw dut::broken(errmsg, "08001");
> 
> > That's clearly the wrong thing to do, but maybe this should be described in the
> > release notes as a compatibility issue, in case other people had the same idea.
> > Clearing errorMessage during success is an option..
> 
> Hm.  I'd debated whether to clear conn->errorMessage at the end of
> a successful connection sequence, and decided not to on the grounds
> that it might be interesting info (eg it could tell you why you
> ended up connected to server Y and not server X).  But perhaps
> it's too much of a compatibility break for this small benefit.

I don't care if applications break because they check the errorMessage instead
of the return value...

..But I think it's not useful to put details into errorMessage on success,
unless you're going to document that.  It would never have occurred to me to
look there, or that it would even be safe.

-- 
Justin