> From: alvhe...@alvh.no-ip.org <alvhe...@alvh.no-ip.org>
> I'll give this another look tomorrow, but I wanted to pass along that I prefer
> libpq-trace.{c,h} instead of libpq-logging. I also renamed variable "pin" and
> pgindented. I don't have any major reservations about this patch now, so I'll
> mark it ready-for-committer in case somebody else wants to have a look
> before push.
>
> (Not sure about the use of the word "forcely")
Hi Iwata-san and Alvaro-san,
Thank you for updating the patch.
Although it's already set as "Ready for Committer", I just found minor parts
that need to be fixed.
(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."
(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."
(3) +PQtraceSetFlags(PGconn *conn, int flags)
+ /* If PQtrace() is failed, do noting. */
"If PQtrace() failed, do nothing."
(4)
> (Not sure about the use of the word "forcely")
I think it's not necessary.
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).
Regards,
Kirk Jamison
