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.ta...@fujitsu.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>></literal> for messages from client to server,
> + and <literal><</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
v29-libpq-trace-log.patch
Description: v29-libpq-trace-log.patch
