Re: Add ldapservice connection parameter

From: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>
To: Andrew Jackson <andrewjackson947(at)gmail(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Roman Khapov <rkhapov(at)yandex-team(dot)ru>, niushiji(at)gmail(dot)com
Subject: Re: Add ldapservice connection parameter
Date: 2026-04-02 14:55:02
Message-ID: 61724fbede15fd95ae29764037fbe71629a8f6d7.camel@cybertec.at
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2026-04-02 14:57:50 Re: Avoid multiple SetLatch() calls in procsignal_sigusr1_handler()
Previous Message Ashutosh Bapat 2026-04-02 14:52:32 Re: Shared hash table allocations