MIME-Version: 1.0
References: 
 <CAKK5BkFOFGfKJNbTuYBvE0PfpHmW8iZEmdNogaCYqjAOhtNgDg@mail.gmail.com>
 <9FF1E1E8-D9EE-43C5-ADFA-7AE94BA57159@yandex-team.ru>
 <CAKK5BkFxWnddC2=mbHpojWnOLe=x3vLftaMUkO3ocJwqZN7Tug@mail.gmail.com>
 <CAKK5BkE_+rCKgZz7+VNNfH_Jm1H6=RcHxg8mmojyR8pu-LOh5w@mail.gmail.com>
 <ba65c0452e7f8218b23f958e4099f98cbabe8bf8.camel@cybertec.at>
 <CAKK5BkH9tp59VP4n0xS0idcrj+HdLJAKzkkiMTsrynyKvYyXFw@mail.gmail.com>
 <3e670a40754bb687d163f21d82af96f059f3d472.camel@cybertec.at>
 <CAKK5BkET2pxuuSvp5ZLsH=LLYS+6JCB+zUAhY38vWsGNJrN6eA@mail.gmail.com>
 <61724fbede15fd95ae29764037fbe71629a8f6d7.camel@cybertec.at>
 <CAKK5BkEsKEnPifH10hXsXt7Z7qN2PPy6i64TH0A9yRr=mT=TtA@mail.gmail.com>
In-Reply-To: 
 <CAKK5BkEsKEnPifH10hXsXt7Z7qN2PPy6i64TH0A9yRr=mT=TtA@mail.gmail.com>
From: Zsolt Parragi <zsolt.parragi@percona.com>
Date: Wed, 8 Apr 2026 08:59:33 +0100
Message-ID: 
 <CAN4CZFOBPOTh=z_OfqqDo1F54vF=2Af7hDKRL=JOHkBprCbmZA@mail.gmail.com>
Subject: Re: Add ldapservice connection parameter
To: Andrew Jackson <andrewjackson947@gmail.com>
Cc: Laurenz Albe <laurenz.albe@cybertec.at>,
 pgsql-hackers <pgsql-hackers@postgresql.org>,
	Roman Khapov <rkhapov@yandex-team.ru>, niushiji@gmail.com,
	Andrey Borodin <x4mmm@yandex-team.ru>
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CAN4CZFOBPOTh%3Dz_OfqqDo1F54vF%3D2Af7hDKRL%3DJOHkBprCbmZA%40mail.gmail.com>
Precedence: bulk

Hello

+ /*
+ * ldapServiceLookup has 4 potential return values. We only care here
+ * if it succeeded, if it failed we dont care why, return failure.
+ */
+ if ((rc = ldapServiceLookup(ldapserviceurl, options, errorMessage)) != 0){
+
+ /*
+ * ldapServiceLookup == 2 is the only return code for libpq_append_error
+ * that does not append error because when used in pg_service.conf it is
+ * allowed to fallback to additional URLs without failing.
+ */
+ if (rc == 2)
+ libpq_append_error(errorMessage,
+            "connection could not be established to ldapserviceurl: \"%s\"",
+    ldapserviceurl);
+
+ return false;

This comment seems to be confusing to me, at first I thought that it
is the opposite of what the code below does, and then I realized that
no, it's just difficult to understand.

Maybe something like:

  /*
   * ldapServiceLookup() return code 2 means the LDAP server could
   * not be contacted. Unlike other non-zero returns, it does not
   * append an error message, because in pg_service.conf parsing
   * the caller silently falls back to the next URL. Here there is
   * no fallback, so we must provide an error message ourselves.
   */

+        This option specifies an LDAP query that can be used to
reference connection parameters
+        stored on an LDAP server. Any connection parameter that is
looked up in this way is
+        overridden by explicitly named connection parameters or
environment variables. This

Is the environment variable part true? ldapServiceLookup is now at
line 6765, environment variables are handled later at 6794 in
conninfo_add_defaults, so it is later, but it also has a NULL check in
it. If a value is already set in ldapServiceLookup, the environment
variable loop later won't override it.