At Tue, 9 Feb 2021 08:10:19 +0000, "tsunakawa.takay@fujitsu.com" <tsunakawa.takay@fujitsu.com> wrote in
> 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.)
Thanks and Yes, I also don't imagine that users change the timestmp
state repeatedly:p It's just a example.
> > > (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.
+1
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
