Re: [HACKERS] Log LDAP "diagnostic messages"?

2017-10-12 Thread Thomas Munro
On Fri, Oct 13, 2017 at 3:59 PM, Peter Eisentraut wrote: > On 9/24/17 07:00, Thomas Munro wrote: >> Fair point. In that case there are a few others we should consider >> moving down too for consistency, like in the attached. > >> Thanks, that is much tidier. Done that way in the attached. >> >>

Re: [HACKERS] Log LDAP "diagnostic messages"?

2017-10-12 Thread Peter Eisentraut
On 9/24/17 07:00, Thomas Munro wrote: > Fair point. In that case there are a few others we should consider > moving down too for consistency, like in the attached. > Thanks, that is much tidier. Done that way in the attached. > > Here also is a small addition to your TAP test which exercises th

Re: [HACKERS] Log LDAP "diagnostic messages"?

2017-09-24 Thread Thomas Munro
On Wed, Sep 20, 2017 at 7:57 AM, Peter Eisentraut wrote: > In the 0001 patch, I would move the ldap_unbind() calls after the > ereport(LOG) calls. We do all the other resource cleanup (pfree() etc.) > after the ereport() calls, so it would be weird to do this one > differently. Also, in the seco

Re: [HACKERS] Log LDAP "diagnostic messages"?

2017-09-19 Thread Peter Eisentraut
On 9/15/17 08:43, Thomas Munro wrote: > On Fri, Sep 15, 2017 at 2:12 AM, Alvaro Herrera > wrote: >> I think the ldap_unbind() changes should be in a separate preliminary >> patch to be committed separately and backpatched. > > OK, here it is split into two patches. I've looked this over. In th

Re: [HACKERS] Log LDAP "diagnostic messages"?

2017-09-18 Thread Alvaro Herrera
Peter Eisentraut wrote: > On 9/14/17 10:21, Alvaro Herrera wrote: > > BTW I added --with-ldap and --with-pam to the configure line for the > > reports in coverage.postgresql.org and the % covered in auth.c went from > > 24% to 18.9% (from very bad to terribly sad). > > You can add src/test/ldap/ n

Re: [HACKERS] Log LDAP "diagnostic messages"?

2017-09-15 Thread Peter Eisentraut
On 9/14/17 10:21, Alvaro Herrera wrote: > BTW I added --with-ldap and --with-pam to the configure line for the > reports in coverage.postgresql.org and the % covered in auth.c went from > 24% to 18.9% (from very bad to terribly sad). You can add src/test/ldap/ now to make up for some of that. --

Re: [HACKERS] Log LDAP "diagnostic messages"?

2017-09-15 Thread Thomas Munro
On Fri, Sep 15, 2017 at 2:12 AM, Alvaro Herrera wrote: > I think the ldap_unbind() changes should be in a separate preliminary > patch to be committed separately and backpatched. OK, here it is split into two patches. > The other bits looks fine, with nitpicks > > 1. please move the new support

Re: [HACKERS] Log LDAP "diagnostic messages"?

2017-09-14 Thread Michael Paquier
On Fri, Sep 15, 2017 at 1:38 AM, Robert Haas wrote: > On Thu, Sep 14, 2017 at 10:21 AM, Alvaro Herrera > wrote: >> BTW I added --with-ldap and --with-pam to the configure line for the >> reports in coverage.postgresql.org and the % covered in auth.c went from >> 24% to 18.9% (from very bad to ter

Re: [HACKERS] Log LDAP "diagnostic messages"?

2017-09-14 Thread Robert Haas
On Thu, Sep 14, 2017 at 10:21 AM, Alvaro Herrera wrote: > BTW I added --with-ldap and --with-pam to the configure line for the > reports in coverage.postgresql.org and the % covered in auth.c went from > 24% to 18.9% (from very bad to terribly sad). Improved code coverage is becoming a fad. --

Re: [HACKERS] Log LDAP "diagnostic messages"?

2017-09-14 Thread Alvaro Herrera
BTW I added --with-ldap and --with-pam to the configure line for the reports in coverage.postgresql.org and the % covered in auth.c went from 24% to 18.9% (from very bad to terribly sad). -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DB

Re: [HACKERS] Log LDAP "diagnostic messages"?

2017-09-14 Thread Alvaro Herrera
I think the ldap_unbind() changes should be in a separate preliminary patch to be committed separately and backpatched. The other bits looks fine, with nitpicks 1. please move the new support function to the bottom of the section dedicated to LDAP, and include a prototype 2. please wrap lines lo

Re: [HACKERS] Log LDAP "diagnostic messages"?

2017-09-14 Thread Ashutosh Bapat
On Wed, Sep 13, 2017 at 6:58 AM, Thomas Munro wrote: > On Tue, Sep 12, 2017 at 11:23 PM, Ashutosh Bapat > wrote: >> On Wed, Aug 16, 2017 at 11:13 AM, Ashutosh Bapat >> wrote: >>> On Wed, Aug 16, 2017 at 8:44 AM, Alvaro Herrera >>> wrote: Christoph Berg wrote: > "Diagnostic message" doe

Re: [HACKERS] Log LDAP "diagnostic messages"?

2017-09-12 Thread Thomas Munro
On Tue, Sep 12, 2017 at 11:23 PM, Ashutosh Bapat wrote: > On Wed, Aug 16, 2017 at 11:13 AM, Ashutosh Bapat > wrote: >> On Wed, Aug 16, 2017 at 8:44 AM, Alvaro Herrera >> wrote: >>> Christoph Berg wrote: "Diagnostic message" doesn't really mean anything, and printing "DETAIL: Diagnostic

Re: [HACKERS] Log LDAP "diagnostic messages"?

2017-09-12 Thread Ashutosh Bapat
The patch needs to address some comments in the previous mails, so marking it as "waiting for author". On Wed, Aug 16, 2017 at 11:13 AM, Ashutosh Bapat wrote: > On Wed, Aug 16, 2017 at 8:44 AM, Alvaro Herrera > wrote: >> Christoph Berg wrote: >>> Re: Thomas Munro 2017-08-10 >>> >>> > > Agreed.

Re: [HACKERS] Log LDAP "diagnostic messages"?

2017-08-15 Thread Ashutosh Bapat
On Wed, Aug 16, 2017 at 8:44 AM, Alvaro Herrera wrote: > Christoph Berg wrote: >> Re: Thomas Munro 2017-08-10 >> >> > > Agreed. Here's a version that skips those useless detail messages >> > > using a coding pattern I found elsewhere. >> > >> > Rebased after bf6b9e94. >> >> > message ? errdetai

Re: [HACKERS] Log LDAP "diagnostic messages"?

2017-08-15 Thread Alvaro Herrera
Christoph Berg wrote: > Re: Thomas Munro 2017-08-10 > > > > Agreed. Here's a version that skips those useless detail messages > > > using a coding pattern I found elsewhere. > > > > Rebased after bf6b9e94. > > > message ? errdetail("Diagnostic message: %s", message) : 0)); > > "Diagnostic mes

Re: [HACKERS] Log LDAP "diagnostic messages"?

2017-08-15 Thread Christoph Berg
Re: Thomas Munro 2017-08-10 > > Agreed. Here's a version that skips those useless detail messages > > using a coding pattern I found elsewhere. > > Rebased after bf6b9e94. > message ? errdetail("Diagnostic message: %s", message) : 0)); "Diagnostic message" doesn't really mean anything, and pr

Re: [HACKERS] Log LDAP "diagnostic messages"?

2017-08-09 Thread Thomas Munro
On Thu, Aug 10, 2017 at 1:16 PM, Ashutosh Bapat wrote: > Please add this to the next commitfest. Done: https://commitfest.postgresql.org/14/1229/ -- Thomas Munro http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscri

Re: [HACKERS] Log LDAP "diagnostic messages"?

2017-08-09 Thread Ashutosh Bapat
On Thu, Aug 10, 2017 at 5:09 AM, Thomas Munro wrote: > On Thu, Jul 27, 2017 at 5:20 PM, Thomas Munro > wrote: >> Agreed. Here's a version that skips those useless detail messages >> using a coding pattern I found elsewhere. > > Rebased after bf6b9e94. > Please add this to the next commitfest.

Re: [HACKERS] Log LDAP "diagnostic messages"?

2017-08-09 Thread Thomas Munro
On Thu, Jul 27, 2017 at 5:20 PM, Thomas Munro wrote: > Agreed. Here's a version that skips those useless detail messages > using a coding pattern I found elsewhere. Rebased after bf6b9e94. -- Thomas Munro http://www.enterprisedb.com ldap-diagnostic-message-v3.patch Description: Binary data

Re: [HACKERS] Log LDAP "diagnostic messages"?

2017-07-26 Thread Thomas Munro
On Wed, Jul 26, 2017 at 10:40 PM, Ashutosh Bapat wrote: > On Wed, Jul 26, 2017 at 6:51 AM, Thomas Munro > wrote: >> Something like the attached. > > The patch prints errdetail() as "No LDAP diagnostic message > available." when LDAP doesn't provide diagnostics. May be some error > messages do not

Re: [HACKERS] Log LDAP "diagnostic messages"?

2017-07-26 Thread Ashutosh Bapat
On Wed, Jul 26, 2017 at 6:51 AM, Thomas Munro wrote: > Hi hackers, > > Some LDAP error codes are a bit vague. For example: > >LDAP_CONNECT_ERROR Indicates a connection problem. >LDAP_PROTOCOL_ERROR A protocol violation was detected. > > To learn more, you have to call > ldap_get_

[HACKERS] Log LDAP "diagnostic messages"?

2017-07-25 Thread Thomas Munro
Hi hackers, Some LDAP error codes are a bit vague. For example: LDAP_CONNECT_ERROR Indicates a connection problem. LDAP_PROTOCOL_ERROR A protocol violation was detected. To learn more, you have to call ldap_get_option(LDAP_OPT_DIAGNOSTIC_MESSAGE). Should we do that? For example