Thread: 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: https://postgr.es/m/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] (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 [1] https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=dory&dt=2021-04-06%2001%3A00%3A07&stg=make
Attachment
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