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