Re: Add ldapservice connection parameter

From: Andrew Jackson <andrewjackson947(at)gmail(dot)com>
To: Andrey Borodin <x4mmm(at)yandex-team(dot)ru>
Cc: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, 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-03-30 17:32:20
Message-ID: CAKK5BkHADztFq3xUv31bQjgJ6-6y4950c=r_0x7t9KFSrD6hvg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andrey,

Thank you for the review.

>I like "ldapservice" approach more. I don't like breaking changes, even for very small number of users.

After thinking about it more it would be pretty easy to make this non
breaking for users. The attached patch
`pgservice-0002-Allow-LDAP-lookup-from-pgservice-connection-paramete.patch`
implements this. Basically if the ldap connection parameter happens to
already exist in pg_service, no LDAP lookup is performed, the values
are instead taken from the pg_service.conf file. I'm fine with either
implementation direction that is the most simple though.

Also attaching the patch
`ldapserviceurl-0005-Add-ldapservice-connection-parameter.patch` which
addresses some issues you found with the previous patch.
- renames parameter from ldapservice to ldapserviceurl
- Adds a comment above the ldapserviceurl call explaining the reason
for ignoring any error info besides zero/non-zero
- Changes dispsize to 64. In reality it maybe should be higher but I
was not comfortable changing it to something like 1024 because it was
an order of magnitude larger than anything else. Just set it to the
current max of 64.
- Also moved the parameter definition and added comments about how it
is defined even on non LDAP builds. This coincides with similar
comments for SSL/GSS specific parameters, though there are no such
comments left for oauth.

Let me know if there is anything else you see missing.

Thanks again,
Andrew Jackson

Attachment Content-Type Size
pgservice-0002-Allow-LDAP-lookup-from-pgservice-connection-paramete.patch text/x-patch 5.6 KB
ldapserviceurl-0005-Add-ldapservice-connection-parameter.patch text/x-patch 7.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Hannu Krosing 2026-03-30 17:32:34 Re: Patch: dumping tables data in multiple chunks in pg_dump
Previous Message Andrey Silitskiy 2026-03-30 17:31:04 Re: Exit walsender before confirming remote flush in logical replication