public inbox for [email protected]  

help / color / mirror / Atom feed

From: Laurenz Albe <[email protected]>
To: Andrew Jackson <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: Roman Khapov <[email protected]>
Cc: [email protected]
Subject: Re: Add ldapservice connection parameter
Date: Thu, 02 Apr 2026 16:55:02 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAKK5BkET2pxuuSvp5ZLsH=LLYS+6JCB+zUAhY38vWsGNJrN6eA@mail.gmail.com>
References: <CAKK5BkFOFGfKJNbTuYBvE0PfpHmW8iZEmdNogaCYqjAOhtNgDg@mail.gmail.com>
	<[email protected]>
	<CAKK5BkFxWnddC2=mbHpojWnOLe=x3vLftaMUkO3ocJwqZN7Tug@mail.gmail.com>
	<CAKK5BkE_+rCKgZz7+VNNfH_Jm1H6=RcHxg8mmojyR8pu-LOh5w@mail.gmail.com>
	<[email protected]>
	<CAKK5BkH9tp59VP4n0xS0idcrj+HdLJAKzkkiMTsrynyKvYyXFw@mail.gmail.com>
	<[email protected]>
	<CAKK5BkET2pxuuSvp5ZLsH=LLYS+6JCB+zUAhY38vWsGNJrN6eA@mail.gmail.com>

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.

> 
> > Good, but you should append something to the "errorMessage", like
> > conninfo_add_defaults() does elsewhere.
> 
> 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.

> 
> 
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) == 0)
> +               /*
> +                * Is this a potential ldapurl or a ldapserviceurl parameter?
> +                */
> +               if (strncmp(line, "ldap:", 5) == 0)
>                 {
>                     int         rc = ldapServiceLookup(line, options, errorMessage);

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

view thread (10+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Add ldapservice connection parameter
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox