pgsql: Improve PQtrace() output format
Improve PQtrace() output format
Transform the PQtrace output format from its ancient (and mostly
useless) byte-level output format to a logical-message-level output,
making it much more usable. This implementation allows the printing
code to be written (as it indeed was) by looking at the protocol
documentation, which gives more confidence that the three (docs, trace
code and actual code) actually match.
Author: 岩田 彩 (Aya Iwata) <iwata.aya@fujitsu.com>
Reviewed-by: 綱川 貴之 (Takayuki Tsunakawa) <tsunakawa.takay@fujitsu.com>
Reviewed-by: Kirk Jamison <k.jamison@fujitsu.com>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: 黒田 隼人 (Hayato Kuroda) <kuroda.hayato@fujitsu.com>
Reviewed-by: "Nagaura, Ryohei" <nagaura.ryohei@jp.fujitsu.com>
Reviewed-by: Ryo Matsumura <matsumura.ryo@fujitsu.com>
Reviewed-by: Greg Nancarrow <gregn4422@gmail.com>
Reviewed-by: Jim Doty <jdoty@pivotal.io>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: /messages/by-id/71E660EB361DF14299875B198D4CE5423DE3FBA4@g01jpexmbkw25
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/198b3716dba68544b55cb97bd120738a86d5df2d
Modified Files
--------------
doc/src/sgml/libpq.sgml | 42 ++-
src/interfaces/libpq/Makefile | 1 +
src/interfaces/libpq/exports.txt | 1 +
src/interfaces/libpq/fe-connect.c | 21 --
src/interfaces/libpq/fe-exec.c | 4 -
src/interfaces/libpq/fe-misc.c | 66 +---
src/interfaces/libpq/fe-protocol3.c | 13 +
src/interfaces/libpq/fe-trace.c | 714 ++++++++++++++++++++++++++++++++++++
src/interfaces/libpq/libpq-fe.h | 17 +-
src/interfaces/libpq/libpq-int.h | 7 +
10 files changed, 804 insertions(+), 82 deletions(-)
On Wed, 31 Mar 2021 at 12:17, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
Improve PQtrace() output format
I see the Visual Studio buildfarm members have been getting a compiler
warning since this commit [1]https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=dory&dt=2021-04-06%2001%3A00%3A07&stg=make
(ClCompile target) ->
src/interfaces/libpq/fe-trace.c(87): warning C4133: 'function':
incompatible types - from 'long *' to 'const time_t *const '
[C:\\pgbuildfarm\\pgbuildroot\\HEAD\\pgsql.build\\libpq.vcxproj]
It looks like this is due to long being 32-bits on Visual Studio.
We could just assign tval.tv_sec to a local time_t value to get rid of
it. See attached.
David
Attachments:
fix_fe-trace_compiler_warning.patchapplication/octet-stream; name=fix_fe-trace_compiler_warning.patchDownload+3-1
David Rowley <dgrowleyml@gmail.com> writes:
I see the Visual Studio buildfarm members have been getting a compiler
warning since this commit [1]
(ClCompile target) ->
src/interfaces/libpq/fe-trace.c(87): warning C4133: 'function':
incompatible types - from 'long *' to 'const time_t *const '
[C:\\pgbuildfarm\\pgbuildroot\\HEAD\\pgsql.build\\libpq.vcxproj]
The current form of it is my fault, I think, see f1be740a9.
Alvaro's commit had different issues ;-(
It looks like this is due to long being 32-bits on Visual Studio.
As best I can tell from these warnings, VS has the tv_sec field
of struct timeval defined as something other than time_t. Because
VS would never stoop to actually complying with the plain text of
the POSIX standard.
We could just assign tval.tv_sec to a local time_t value to get rid of
it. See attached.
Yeah, I've not got a better idea. But please add some comment
about how this is working around VS brain-damage, because anyone
looking at the available standards documents would think the
copying is unnecessary.
regards, tom lane
I wrote:
As best I can tell from these warnings, VS has the tv_sec field
of struct timeval defined as something other than time_t. Because
VS would never stoop to actually complying with the plain text of
the POSIX standard.
Ah, seems like it's really Winsock's fault:
https://docs.microsoft.com/en-us/windows/win32/api/winsock/ns-winsock-timeval
which compares unfavorably to the POSIX wording
The <sys/time.h> header shall define the timeval structure, which
shall include at least the following members:
time_t tv_sec Seconds.
suseconds_t tv_usec Microseconds.
regards, tom lane
On Tue, 6 Apr 2021 at 17:21, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I wrote:
As best I can tell from these warnings, VS has the tv_sec field
of struct timeval defined as something other than time_t. Because
VS would never stoop to actually complying with the plain text of
the POSIX standard.Ah, seems like it's really Winsock's fault:
https://docs.microsoft.com/en-us/windows/win32/api/winsock/ns-winsock-timeval
Yeah. Thanks for having a look. I pushed the fix with a comment
explaining why we bother with the local variable.
David