RE: libpq debug log

Hi Tsunakawa san,

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

Output is following. It is fine.
```
2021-03-19 07:21:09.917302 > 4 Terminate
2021-03-19 07:21:09.961494 > 155 Query "CREATE TABLESPACE regress_tblspacewith LOCATION '/home/iwata/pgsql/postgres/src/test/regress/testtablespace' WITH (some_nonexistent_parameter = true);"
2021-03-19 07:21:09.962657 < 124 ErrorResponse S "ERROR" V "ERROR" C "22023" M "unrecognized parameter "some_nonexistent_parameter"" F "reloptions.c" L "1456" R "parseRelOptionsInternal" \x00 "Z"
2021-03-19 07:21:09.962702 < 5 ReadyForQuery I
2021-03-19 07:21:09.962767 > 144 Query "CREATE TABLESPACE regress_tblspacewith LOCATION '/home/iwata/pgsql/postgres/src/test/regress/testtablespace' WITH (random_page_cost = 3.0);"
2021-03-19 07:21:09.967056 < 22 CommandComplete "CREATE TABLESPACE"
2021-03-19 07:21:09.967092 < 5 ReadyForQuery I
2021-03-19 07:21:09.967148 > 81 Query "SELECT spcoptions FROM pg_tablespace WHERE spcname = 'regress_tblspacewith';"
2021-03-19 07:21:09.970338 < 35 RowDescription 1 "spcoptions" 1213 5 1009 65535 -1 0
2021-03-19 07:21:09.970402 < 32 DataRow 1 22 '{random_page_cost=3.0}'
2021-03-19 07:21:09.970420 < 13 CommandComplete "SELECT 1"
2021-03-19 07:21:09.970431 < 5 ReadyForQuery I
2021-03-19 07:21:09.970558 > 42 Query "DROP TABLESPACE regress_tblspacewith;"
2021-03-19 07:21:09.971500 < 20 CommandComplete "DROP TABLESPACE"
2021-03-19 07:21:09.971561 < 5 ReadyForQuery I
```

> -----Original Message-----
> From: Tsunakawa, Takayuki/綱川 貴之 <tsunakawa(dot)takay(at)fujitsu(dot)com>
> Sent: Friday, March 19, 2021 11:37 AM

> Thanks to removing the static arrays, the code got much smaller. I'm happy
> with this result.

Thank you so much. Your advice was very helpful in refactoring the patch.

> (1)
> + (<literal>&gt;</literal> for messages from client to server,
> + and <literal>&lt;</literal> for messages from server to client),
>
> I think this was meant to say "> or <". So, maybe you should remove "," at
> the end of the first line, and replace "and" with "or".

I fixed.

>
>
> (2)
> + case 8 : /* GSSENCRequest or SSLRequest */
> + /* These messsage does not reach here. */
>
> messsage does -> messages do

I fixed.

> (3)
> + fprintf(f, "\tAuthentication");
>
> Why don't you move this \t in each message type to the end of:
>
> + fprintf(conn->Pfdebug, "%s\t%s\t%d", timestr, prefix, length);

I fixed.

>
> Plus, don't we say in the manual that fields (timestamp, direction, length,
> message type, and message content) are separated by a tab?

Sure. I added following documentation;
+ Non-message contents fields (timestamp, direction, length and message type)
+ are separated by a tab. Message contents are separated by a space.

> Also, the code doesn't seem to separate the message type and content with a
> tab.

I fixed to output a tab at the end of message types.

> (4)
> Where did the processing for unknown message go in
> pqTraceOutputMessage()? Add default label?

> (5)
> + case 16: /* CancelRequest */
> + fprintf(conn->Pfdebug,
> "%s\t>\t%d\tCancelRequest", timestr, length);
>
> I think this should follow pqTraceOutputMessage(). That is, each case label
> only print the message type name in case other message types are added in
> the future.

Sure. I fixed pqTraceOutputNoTypeByteMessage() following to pqTraceOutputMessage() style.

Regards,
Aya Iwata