Horton

RE: libpq debug log

Aya Iwata (Fujitsu) <iwata.aya@fujitsu.com>

View thread
From: "iwata.aya@fujitsu.com" <iwata.aya@fujitsu.com>
To: "k.jamison@fujitsu.com" <k.jamison@fujitsu.com>, "'alvherre@alvh.no-ip.org'" <alvherre@alvh.no-ip.org>
Cc: "tsunakawa.takay@fujitsu.com" <tsunakawa.takay@fujitsu.com>, 'Kyotaro Horiguchi' <horikyota.ntt@gmail.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Date: 2021-02-24T10:28:39Z
Lists: pgsql-hackers

Attachments

Hi Kirk san,

Thank you for your review. I update patch to v21.

> -----Original Message-----
> From: Jamison, Kirk/ジャミソン カーク <k.jamison@fujitsu.com>
> Sent: Wednesday, February 24, 2021 1:04 PM


> (1) Doc: PQtraceSetFlags
> +      <literal>flags</literal> contains flag bits describing the operating
> mode
> +      of tracing.  If <literal>flags</literal> contains
> <literal>PQTRACE_SUPPRESS_TIMESTAMPS</literal>,
> +      then timestamp is not printed with each message. If set to 0
> (default),tracing will be output with timestamp.
> +      This function should be called after calling
> <function>PQtrace</function>.
> 
> Missing space. And can be paraphrased to:
> "If set to 0 (default), tracing with timestamp is printed."

I fixed this documentation as you suggested.

> (2)
> + * pqTraceMaybeBreakLine:
> + *             Check whether the backend message is complete. If so, print
> a line
> + *             break and reset the buffer. If print break line, return 1.
> 
> The 2nd & last sentence can be combined to  "If so, print a line break, reset
> the buffer, and return 1."

I fixed it because it is more natural than previous explanation.


> (3) +PQtraceSetFlags(PGconn *conn, int flags)
> +	/* If PQtrace() is failed, do noting. */
> 
> "If PQtrace() failed, do nothing."

I fixed it.

 
> (4)
> > (Not sure about the use of the word "forcely")
> 
> I think it's not necessary.

Sure. 

> Also, I tested the flag to not print timestamp. For example,
>  PQtrace(conn, trace_file);
>  PQtraceSetFlags(conn, PQTRACE_SUPPRESS_TIMESTAMPS);
> 
> And it did not print the timestamp. So it worked.
> It also passed all the regression tests. (although PQtrace() is not tested in
> existing libpq tests).

Thank you for your test.

Regards,
Aya Iwata
Fujitsu