From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
> > (45)
> This looks like a fusion of PQtrace and PQtraceEX. By the way, the
> timestamp flag is needed at log emittion. So we can change the state
> anytime.
>
> PQtrace(conn, of);
> PQtraceSetFlags(conn, PQTRACE_SUPPRESS_TIMESTAMPS);
> <logging without timestamps>
> PQtraceSetFlags(conn, 0);
> <logging with timestamps>
I find this better because the application does not have to call PQuntrace() and PQtrace() again to enable/disable
timestampoutput, which requires passing the FILE pointer again. (I don't imagine applications would repeatedly turn
loggingon and off in practice, though.)
> > (46)
> >
> > If skipLogging is intended for use with backend -> frontend messages only,
> shouldn't it be placed in conn->b_msg?
>
> The name skipLogging is somewhat obscure. The flag doesn't inhibit all
> logs from being emitted. It seems like it represents how far bytes
> the logging mechanism consumed for the limited cases. Thus, I think it
> can be a cursor variable like inCursor.
>
> If we have conn->be_msg->inLogged, for example, pqGetc and
> pqLogMessageByte1() are written as the follows.
>
> pqGetc(char *result, PGconn *conn)
> {
> if (conn->inCursor >= conn->inEnd)
> return EOF;
>
> *result = conn->inBuffer[conn->inCursor++];
>
> if (conn->Pfdebug)
> pqLogMessageByte1(conn, *result);
>
> return 0;
> }
>
> pqLogMessageByte1(...)
> {
> switch()
> {
> case LOG_FIRST_BYTE:
> /* No point in logging already logged bytes */
> if (conn->be_msg->inLogged >= conn->inCursor)
> return;
> ...
> }
> conn->be_msg->inLogged = conn->inCursor;
> }
This looks better design because stuff like skipLogging is an internal state of logging facility whose use should be
restrictedto fe-logging.c.
> (pqCheckInBufferSpace needs to adjust inLogged.)
>
> I'm not sure this is easier to read than the skipLogging.
I'd like Iwata-san to evaluate this and decide whether to take this approach or the current one.
Regards
Takayuki Tsunakawa
