Horton

Re: libpq debug log

Greg Nancarrow <gregn4422@gmail.com>

View thread View on postgr.es
From: Greg Nancarrow <gregn4422@gmail.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: "iwata.aya@fujitsu.com" <iwata.aya@fujitsu.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>, "tgl@sss.pgh.pa.us" <tgl@sss.pgh.pa.us>, "robertmhaas@gmail.com" <robertmhaas@gmail.com>, "pchampion@pivotal.io" <pchampion@pivotal.io>, "jdoty@pivotal.io" <jdoty@pivotal.io>, "raam.soft@gmail.com" <raam.soft@gmail.com>, "nagaura.ryohei@fujitsu.com" <nagaura.ryohei@fujitsu.com>, "nagata@sraoss.co.jp" <nagata@sraoss.co.jp>, "peter.eisentraut@2ndquadrant.com" <peter.eisentraut@2ndquadrant.com>, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>, "k.jamison@fujitsu.com" <k.jamison@fujitsu.com>
Date: 2020-12-15T09:04:55Z
Lists: pgsql-hackers 
On Tue, Oct 27, 2020 at 3:23 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> I've been hacking at this patch again.  There were a few things I wasn't
> too clear about, so I reordered the code and renamed the routines to try
> to make it easier to follow.
>

Hi,

Hopefully Iwata-san will return to looking at this soon.

I have tried the latest patch a little (using "psql" as my client),
and have a few small comments so far:

- In pqTraceInit(), in the (admittedly rare) case that fe_msg malloc()
fails, I'd NULL out be_msg too after free(), rather than leave it
dangling (because if pgTraceInit() was ever invoked again, as the
comment says it could, it would result in previously freed memory
being accessed ...)

conn->fe_msg = malloc(MAX_FRONTEND_MSGS * sizeof(pqFrontendMessage));
if (conn->fe_msg == NULL)
{
    free(conn->be_msg);
    conn->be_msg = NULL;
    return false;
}

- >3. COPY ... (FORMAT BINARY) emits "invalid protocol" ... not good.

That seemed to happen for me only if COPYing binary format to stdout.

    UnknownCommand :::Invalid Protocol

- >5. Error messages are still printing the terminating zero byte.  I
  >suggest that it should be suppressed.

Perhaps there's a more elegant way of doing it, but I got rid of the
logging of the zero byte using the following change to
pgLogMsgByte1(), though there still seems to be a trailing space
issue:

                        case LOG_CONTENTS:
-                               fprintf(conn->Pfdebug, "%c ", v);
+                               if (v != '\0')
+                                   fprintf(conn->Pfdebug, "%c ", v);
                                pqTraceMaybeBreakLine(sizeof(v), conn);
                                break;

Regards,
Greg Nancarrow
Fujitsu Australia

Commits

  1. Rename PQtraceSetFlags() to PQsetTraceFlags().

  2. Suppress length of Notice/Error msgs in PQtrace regress mode

  3. Strip file names reported in error messages on Windows, too.

  4. Fix setvbuf()-induced crash in libpq_pipeline

  5. libpq_pipeline: Must strdup(optarg) to avoid crash

  6. Remove setvbuf() call from PQtrace()

  7. Initialize conn->Pfdebug to NULL when creating a connection

  8. Disable force_parallel_mode in libpq_pipeline

  9. libpq_pipeline: add PQtrace() support and tests

  10. Improve PQtrace() output format

  11. Re-simplify management of inStart in pqParseInput3's subroutines.