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: 'Tom Lane' <tgl@sss.pgh.pa.us>, Alvaro Herrera <alvherre@alvh.no-ip.org>, 'Kyotaro Horiguchi' <horikyota.ntt@gmail.com>, "k.jamison@fujitsu.com" <k.jamison@fujitsu.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Date: 2021-03-16T07:40:38Z
Lists: pgsql-hackers 
I've finished the review.  Here are the last set of comments.


(16)
(15) is also true for the processing of 'H' message.


(17) pqTraceOutputD
+		for (i = 0; i < nfields; i++)
+		{
+			len = pqTraceOutputInt32(message + cursor, f);
+			cursor += 4;
+			if (len == -1)
+				break;
+			pqTraceOutputNchar(message + cursor, f, len);
+			cursor += len;
+		}

Not break but continue, because non-NULL columns may follow a NULL column.


(18)
+		case 'E':	/* Execute(F) or Error Return(B) */
+			pqTraceOutputE(message + LogCursor, LogEnd, conn->Pfdebug, toServer);
+			break;

Error Return -> ErrorResponse


(19) pqTraceOutputF
Check the protocol again.  Two for loops should be required, one for format codes and another for arguments.


(20)
+	if ( len != -1)

Remove extra space before len.


(21)
I guess you intentionally omitted the following messages because they are only used during connection establishment.  I'm fine with it.  I wrote this just in case you missed them.

GSSENCRequest (F)
Int32(8)

GSSResponse (F)
Byte1('p')

PasswordMessage (F)
Byte1('p')

SASLInitialResponse (F)
Byte1('p')

SASLResponse (F)
Byte1('p')


(22)
+/* NotificationResponse */
+static void
+pqTraceOutputA(const char *message, int end, FILE *f)
+{
+	int	cursor = 0;
+
+	pqTraceOutputInt16(message + cursor, f);
+	cursor += 2;
+	pqTraceOutputString(message + cursor, f);

Not Int16 but Int32.


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.