Date: Sat, 21 Jan 2023 23:14:08 -0800
From: Noah Misch <noah@leadboat.com>
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@postgresql.org
Subject: Re: libpqrcv_connect() leaks PGconn
Message-ID: <20230122071408.GB1400215@rfd.leadboat.com>
References: <20230121011237.q52apbvlarfv6jm6@awork3.anarazel.de>
 <20230121025037.mildbmgylv4ybp4a@awork3.anarazel.de>
 <20230121161642.GD1356297@rfd.leadboat.com>
 <20230121200453.ba7eftju7hwaf3cd@awork3.anarazel.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230121200453.ba7eftju7hwaf3cd@awork3.anarazel.de>
User-Agent: Mutt/1.5.24 (2015-08-30)
Archived-At: <https://www.postgresql.org/message-id/20230122071408.GB1400215%40rfd.leadboat.com>
Precedence: bulk

On Sat, Jan 21, 2023 at 12:04:53PM -0800, Andres Freund wrote:
> On 2023-01-21 08:16:42 -0800, Noah Misch wrote:
> > On Fri, Jan 20, 2023 at 06:50:37PM -0800, Andres Freund wrote:
> > > I seems we don't have any tests for creating a subscription that fails during
> > > connection establishment? That doesn't seem optimal - I guess there may have
> > > been concern around portability of the error messages?
> > 
> > Perhaps.  We have various (non-subscription) tests using "\set VERBOSITY
> > sqlstate" for that problem.  If even the sqlstate varies, a DO block is the
> > next level of error swallowing.
> 
> That's a good trick I need to remember. And the errcode for an invalid
> connection string luckily differs from the one for a not working one.
> 
> 
> I think found an even easier way - port=-1 is rejected during PQconnectPoll()
> and will never even open a socket. That'd make it reasonable for the test to
> happen in subscription.sql, instead of a tap test, I think (faster, easier to
> maintain). It may be that we'll one day move that error into the
> PQconninfoParse() phase, but I don't think we need to worry about it now.
> 
> Any reason not to go for that?

No, a port=-1 test in subscription.sql sounds ideal.

> If not, I'll add a test for an invalid conninfo and a non-working connection
> string to subscription.sql.