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.

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

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

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.

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/

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.

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

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

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

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

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,

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:

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

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

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

2017-08-15 Thread Alvaro Herrera
Christoph Berg wrote: > Re: Thomas Munro 2017-08-10 >

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

2017-08-15 Thread Christoph Berg
Re: Thomas Munro 2017-08-10

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

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

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

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

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

[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