Date: Thu, 5 Sep 2019 18:21:22 -0400
From: Alvaro Herrera from 2ndQuadrant <alvherre@alvh.no-ip.org>
To: "Iwata, Aya" <iwata.aya@jp.fujitsu.com>
Cc: "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" <nagaura.ryohei@fujitsu.com>,
	"nagata@sraoss.co.jp" <nagata@sraoss.co.jp>,
	"kommi.haribabu@gmail.com" <kommi.haribabu@gmail.com>,
	"peter.eisentraut@2ndquadrant.com" <peter.eisentraut@2ndquadrant.com>,
	'Kyotaro HORIGUCHI' <horiguchi.kyotaro@lab.ntt.co.jp>,
	"Jamison, Kirk" <k.jamison@jp.fujitsu.com>
Subject: Re: libpq debug log
Message-ID: <20190905222122.GA29713@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <71E660EB361DF14299875B198D4CE5423DFBBFCB@g01jpexmbkw25>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

Hello,

Nice patch.  I think there's pretty near consensus that this is
something we want, and that the output format of one trace msg per libpq
msg is roughly okay.  (I'm not sure there's 100% agreement on this last
point, but it seems close enough.)

> I changed like this:
> 
> 2019-04-04 02:39:51.488 UTC  > Query 59 "SELECT pg_catalog.set_config('search_path', '', false)"

The "59" there seems quite odd, though.

* What is this in pg_conn?  I don't understand why this array is of size
  MAXPGPATH.
+	PGFrontendLogMsgEntry frontend_entry[MAXPGPATH];
  This seems to make pg_conn much larger than we'd like.

Minor review points:

* Do we need COMMAND_B_MIN etc to reduce the number of zeroes at the
beginning of the tables?

* The place where you define TRACELOG_TIME_SIZE seems like it'll be
  unavoidably out of date if somebody changes the format string.  I'd put
  that #define next to where the fmt appears.

* "output a a null-terminated" has duplicated "a"

* We don't add braces around single-statement blocks (pqPutMsgEnd)

* Please pgindent.


Thanks

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services