Message-ID: <61724fbede15fd95ae29764037fbe71629a8f6d7.camel@cybertec.at>
Subject: Re: Add ldapservice connection parameter
From: Laurenz Albe <laurenz.albe@cybertec.at>
To: Andrew Jackson <andrewjackson947@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>, Roman Khapov
	 <rkhapov@yandex-team.ru>, niushiji@gmail.com
Date: Thu, 02 Apr 2026 16:55:02 +0200
In-Reply-To: 
 <CAKK5BkET2pxuuSvp5ZLsH=LLYS+6JCB+zUAhY38vWsGNJrN6eA@mail.gmail.com>
References: 
	<CAKK5BkFOFGfKJNbTuYBvE0PfpHmW8iZEmdNogaCYqjAOhtNgDg@mail.gmail.com>
	 <9FF1E1E8-D9EE-43C5-ADFA-7AE94BA57159@yandex-team.ru>
	 <CAKK5BkFxWnddC2=mbHpojWnOLe=x3vLftaMUkO3ocJwqZN7Tug@mail.gmail.com>
	 <CAKK5BkE_+rCKgZz7+VNNfH_Jm1H6=RcHxg8mmojyR8pu-LOh5w@mail.gmail.com>
	 <ba65c0452e7f8218b23f958e4099f98cbabe8bf8.camel@cybertec.at>
	 <CAKK5BkH9tp59VP4n0xS0idcrj+HdLJAKzkkiMTsrynyKvYyXFw@mail.gmail.com>
	 <3e670a40754bb687d163f21d82af96f059f3d472.camel@cybertec.at>
	 <CAKK5BkET2pxuuSvp5ZLsH=LLYS+6JCB+zUAhY38vWsGNJrN6eA@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.58.3 (3.58.3-1.fc43) 
MIME-Version: 1.0
Archived-At: 
 <https://www.postgresql.org/message-id/61724fbede15fd95ae29764037fbe71629a8f6d7.camel%40cybertec.at>
Precedence: bulk

On Wed, 2026-04-01 at 18:07 -0500, Andrew Jackson wrote:
> Attached is an updated patch.

Thank you, it applies and passes the regression tests.

>=20
> > Good, but you should append something to the "errorMessage", like
> > conninfo_add_defaults() does elsewhere.
>=20
> Added an error append in this patch. It has the impact of printing 2
> lines of errors on failure though: one from the newly appended message
> and one from ldapServiceLookup. Not sure if there are other examples
> of this behavior in libpq. Will look into this  more tommorow.

Oh, I didn't look at that.  If ldapServiceLookup() already appends a
meaningful error message, there is no need to add another one.

>=20
>=20
The documentation is better now.

> --- a/src/interfaces/libpq/fe-connect.c
> +++ b/src/interfaces/libpq/fe-connect.c
>  #ifdef USE_LDAP
> -               if (strncmp(line, "ldap", 4) =3D=3D 0)
> +               /*
> +                * Is this a potential ldapurl or a ldapserviceurl parame=
ter?
> +                */
> +               if (strncmp(line, "ldap:", 5) =3D=3D 0)
>                 {
>                     int         rc =3D ldapServiceLookup(line, options, e=
rrorMessage);

That change does not really belong to the patch, but is alright by me.

I'll mark the patch as "ready for committer".

Yours,
Laurenz Albe