RE: libpq debug log

Hi all,

I update patch to v18. It has been fixed in response to Tsunakawa san's review.

> From: Tsunakawa, Takayuki/綱川 貴之 <tsunakawa(dot)takay(at)fujitsu(dot)com>
> Sent: Friday, February 12, 2021 1:53 PM
>
> (48)
> <synopsis>
> void PQtrace(PGconn *conn, FILE *stream);
> </synopsis>
> </para>
>
> + <para>
> + Calls <function>PQtraceSetFlags</function> to output with or
> without a timestamp.
> + </para>
> +
> <note>
>
> Why is this necessary? Even if you decide to remove this change, can you
> share your idea on why you added this just in case?

In the previous patch, we described the PQTRACE_SUPPRESS_TIMESTAMPS flag in the PQtrace () part.
And I think PQtrace () and PQtraceSetFlag () are closely related functions.
It would be nice for the user to know both features before using the logging feature.
So I added a description of PQtraceSetFlag () to the paragraph of PQtrace ().
However, the PQtraceSetFlag () documentation is in the next paragraph of PQtrace (),
so users can easily notice the PQtraceSetFlag () function. So I removed it.

> (49)
> + Determine to output tracing with or without a timestamp to a
> debugging file stream.
>
> The description should be the overall functionality of this function considering
> the future additions of flags. Something like:
>
> Controls the tracing behavior of client/server communication.
>
> + then timestamp is not printed with each message. The default is
> output with timestamp.
>
> The default value for flags argument (0) should be described here.
>
> Also, it should be added that PQsetTraceFlags() can be called before or after
> the PQtrace() call.

I fixed documentation like this;
If set to 0 (default),tracing will be output with timestamp.
This function should be called after calling <function>PQtrace</function>.

> (50)
> I'd like you to consider skipLogging again, because it seems that such a
> variable is for the logging machinery to control state transitions internally in
> fe-logging.c.
> What I'm concerned is that those who modify libpq have to be aware of
> skipLogging and would fail to handle it.

To implement inLogging, logging skip method has been modified to be completed within the libpq-logging.c file.
The protocol message consists of First Byte, Length, and Contents.
When the first byte and length arrive, if contents is not yet in the buffer,
the process of reading the first byte and length is repeated.
Reloading is necessary to parse the protocol message and proceed,
but we want to skip logging because it is not necessary to output the message which already log to file.

To do this, remember the value of inCursor in the inLogging variable and record how far logging has progressed.
In the pqTraceLogBeMsgByte1() and pqTraceLogBeMsgInt(),
the value of inCursor at that time is stored in inLogging after outputting the log to a file.
If inCursor is smaller than inLogging, it exits the log output function without doing anything.
If all log messages are output, initialize the inLogging.

Changed the pqTraceMaybeBreakLine() function to return whether a line break has occurred
in order to consolidate the process of recording the cursor in one place. If it break line,
inLogging is initialized and inCursor is not remembered inLogging.

Also, as Horiguchi san mentioned, I changed to update the value of inLogging
even in the function where the value of inCursor is changed (Ex. pqCheckInBufferSpace () and pqReadData ()).

> (51)
> >> +typedef enum PGLogState
> >> +{
> >>
> >> This is libpq-logging.c internal type. It is not needed to be exposed.
>
> > I fixed it.
>
> How did you fix it? The typedef is still in .h file. It should be in .h, shouldn't
> it? Houw about the other typedefs?

I didn’t move PGLogState to .c file because PGLogState is a member of be_msg which is referred from other files.

Regards,
Aya Iwata
Fujitsu