Horton

RE: libpq debug log

tsunakawa.takay@fujitsu.com <tsunakawa.takay@fujitsu.com>

View thread View on postgr.es
From: "tsunakawa.takay@fujitsu.com" <tsunakawa.takay@fujitsu.com>
To: "iwata.aya@fujitsu.com" <iwata.aya@fujitsu.com>
Cc: 'Kyotaro Horiguchi' <horikyota.ntt@gmail.com>, "alvherre@alvh.no-ip.org" <alvherre@alvh.no-ip.org>, "tgl@sss.pgh.pa.us" <tgl@sss.pgh.pa.us>, "k.jamison@fujitsu.com" <k.jamison@fujitsu.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Date: 2021-03-19T02:37:14Z
Lists: pgsql-hackers 
Hello Iwata-san,


Thanks to removing the static arrays, the code got much smaller.  I'm happy with this result.


(1)
+      (<literal>&gt;</literal> for messages from client to server,
+      and <literal>&lt;</literal> for messages from server to client),

I think this was meant to say "> or <".  So, maybe you should remove "," at the end of the first line, and replace "and" with "or".


(2)
+		case 8 :	/* GSSENCRequest or SSLRequest */
+			/* These messsage does not reach here. */

messsage does -> messages do


(3)
+	fprintf(f, "\tAuthentication");

Why don't you move this \t in each message type to the end of:

+	fprintf(conn->Pfdebug, "%s\t%s\t%d", timestr, prefix, length);

Plus, don't we say in the manual that fields (timestamp, direction, length, message type, and message content) are separated by a tab?
Also, the code doesn't seem to separate the message type and content with a tab.


(4)
Where did the processing for unknown message go in pqTraceOutputMessage()?  Add default label?


(5)
+		case 16:	/* CancelRequest */
+			fprintf(conn->Pfdebug, "%s\t>\t%d\tCancelRequest", timestr, length);

I think this should follow pqTraceOutputMessage().  That is, each case label only print the message type name in case other message types are added in the future.



Regards
Takayuki Tsunakawa

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.