From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
cc: Justin Pryzby <pryzby@telsasoft.com>, 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
In-reply-to: <YMAeyrIW/4Zuq5PZ@paquier.xyz>
References: <YLSCrAPVfxX3X8ap@paquier.xyz> <2566986.1622468180@sss.pgh.pa.us>
 <YLWFzzrUIy9Q8h1d@paquier.xyz> <2732890.1622509658@sss.pgh.pa.us>
 <YLXjFOV3teAPirmS@paquier.xyz> <744012.1623014869@sss.pgh.pa.us>
 <YL3AHgBeUoFPM6lT@paquier.xyz> <869606.1623076683@sss.pgh.pa.us>
 <YL7i73c8vDGD+Gjy@paquier.xyz> <1086565.1623165694@sss.pgh.pa.us>
 <YMAeyrIW/4Zuq5PZ@paquier.xyz>
Comments: In-reply-to Michael Paquier <michael@paquier.xyz>
	message dated "Wed, 09 Jun 2021 10:52:10 +0900"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1261965.1623206528.1@sss.pgh.pa.us>
Date: Tue, 08 Jun 2021 22:42:08 -0400
Message-ID: <1261966.1623206528@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1261966.1623206528%40sss.pgh.pa.us>
Precedence: bulk

Michael Paquier <michael@paquier.xyz> writes:
> On Tue, Jun 08, 2021 at 11:21:34AM -0400, Tom Lane wrote:
>> IIUC, what you are proposing to do is replace pgwin32_setenv with
>> src/port/setenv.c.  I don't think that's an improvement.  setenv.c
>> leaks memory on repeat calls, because it cannot know what
>> pgwin32_setenv knows about how putenv works on that platform.

> Is gaur the only animal that needs this file, by the way?

I think it is.  setenv has been in POSIX for awhile, so probably
only very old systems would need that.  (This is why I don't care
that much that setenv.c leaks memory.  But we can't start using it
on platforms where we *do* care about performance.)

>> (3) Don't try to use the environment variable for this
>> purpose.  I'd originally tried to change test5.pgc to just
>> specify gssmode=disable in-line, but that only works
>> nicely for one of the two failing cases.  The other one
>> is testing the case of a completely defaulted connection
>> target, so there's no place to add an option without
>> breaking the only unique aspect of that test case.

> FWIW, I'd be rather in favor of doing (3) because this remains simple
> just to take care of an edge case, even if that partially breaks the
> promise to rely on a default connection.

Yeah, it doesn't seem like we need to test that case all that
badly.  I'd be okay with dropping that test; or maybe we could
fix things so that the default case succeeds?

			regards, tom lane