| 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
| 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 |