Hi Alvaro san, Thank you for your email. I will review this updated patch and I will let you know when I complete. Please wait a moment.
Best regards, Aya Iwata > -----Original Message----- > From: Alvaro Herrera <alvhe...@alvh.no-ip.org> > Sent: Tuesday, October 27, 2020 1:23 AM > To: Iwata, Aya/岩田 彩 <iwata....@fujitsu.com> > Cc: pgsql-hack...@postgresql.org; t...@sss.pgh.pa.us; > robertmh...@gmail.com; pchamp...@pivotal.io; jd...@pivotal.io; > raam.s...@gmail.com; Nagaura, Ryohei/永浦 良平 > <nagaura.ryo...@fujitsu.com>; nag...@sraoss.co.jp; > peter.eisentr...@2ndquadrant.com; 'Kyotaro HORIGUCHI' > <horiguchi.kyot...@lab.ntt.co.jp>; Jamison, Kirk/ジャミソン カーク > <k.jami...@fujitsu.com> > Subject: Re: libpq debug log > > Hello Aya Iwata > > I've been hacking at this patch again. There were a few things I wasn't too > clear about, so I reordered the code and renamed the routines to try to make > it > easier to follow. > > One thing I didn't like very much is that all the structures and enums were > exposed to the world in libq-int.h. Because some enum members have > pretty generic names, I didn't like that much, so I moved the whole thing to > fe-misc.c, and renamed the structs. Also, the arrays don't take space unless > PQtrace() is called. (This is not what I was talking about in my previous > message. The idea I was trying to explain in my previous message cannot > possibly work, so I abandoned it.) > > I also renamed functions to make it clear which handles frontend and which > handles backend. With that, it was pretty obvious that we had an "reset BE > message" in the routine to handle FE, and some clearing of FE in the code that > handles BE. I fixed things in a way that I think makes the most sense. > > I noticed that it's theoretically possible to have a FE message so large (more > than MAXPGPATH pieces) that it would overrun the array; so I added a "print > message" call after adding one piece, to avoid this. Also, why MAXPGPATH? > I added a new symbol MAX_FRONTEND_MSGS for this purpose. > > There are some things still to do: > > 0. I added a XXX comment to pqFlush. Because we're storing messages in > fe_msgs that point to the libpq buffer, is it possible to end up with trace > messages that are pointing into outBuffer bytes that are already sent, and > perhaps even overwritten with newer bytes? Maybe not, but it's unclear. > Should we do pqLogFrontendMsg() preventively to avoid this problem? > > 1. Is the handling of protocol 2 correct? Since it's completely separate from > protocol 3, I have not even looked at what it produces. > But the fact that pqLogMsgByte1 completely ignores the "commsource" > argument makes me suspect that it's not correct. > 1a. How do we even test protocol 2 handling? > > 2. We need a mode to suppress print of time; this would be useful to be able > to > test libpq at a low level. Maybe instead of time we can print a > monotonically-increasing packet sequence number. With this, we could > easily add tests for libpq itself. What user interface do we want for this? > Maybe we can add an "bits32 flags" parameter to PQtrace(), with one bit for > this use. > > 3. COPY ... (FORMAT BINARY) emits "invalid protocol" ... not good. > > 4. Even in text format, COPY output is not very useful. How can we improve > that? > > 5. Error messages are still printing the terminating zero byte. I suggest > that > it should be suppressed. > > 6. Let's redefine pqTraceMaybeBreakLine() (I renamed from > pqLogLineBreak): If message is complete, print a newline; if message is not > complete, print a " ". Then, remove the whitespace after printing each > element. This should lead to lines that don't have an useless " " > at the end. > > 7. Why does it make sense to call pqTraceMaybeBreakLine when > commsource=FROM_FRONTEND?
