From: Kyotaro Horiguchi
> Using (inCursor - inStart) as logCursor doesn't work correctly if tracing
> state
> desyncs. Once desync happens inStart can be moved at the timing that the
> tracing code doesn't expect. This requires (as I mentioned upthread)
> pqReadData to actively reset logCursor,
At Fri, 26 Feb 2021 17:30:32 +0900 (JST), Kyotaro Horiguchi
wrote in
> At Fri, 26 Feb 2021 16:12:39 +0900 (JST), Kyotaro Horiguchi
> wrote in
> > This inhibits logCursor being updated. What is worse, I find that
> > logCursor movement is quite dubious.
>
> Using (inCursor - inStart) as logCu
At Fri, 26 Feb 2021 16:12:39 +0900 (JST), Kyotaro Horiguchi
wrote in
> This inhibits logCursor being updated. What is worse, I find that
> logCursor movement is quite dubious.
Using (inCursor - inStart) as logCursor doesn't work correctly if
tracing state desyncs. Once desync happens inStart c
At Fri, 26 Feb 2021 06:21:15 +, "tsunakawa.ta...@fujitsu.com"
wrote in
> From: Álvaro Herrera
> > It appears that something is still wrong. I applied lipq pipeline v27 from
> > [1] and ran src/test/modules/test_libpq/pipeline singlerow, after patching
> > it to do PQtrace() after PQconn(
From: Álvaro Herrera
> It appears that something is still wrong. I applied lipq pipeline v27 from
> [1] and ran src/test/modules/test_libpq/pipeline singlerow, after patching it
> to do PQtrace() after PQconn(). Below is the output I get from that. The
> noteworthy point is that "ParseComplete
(Is what I'm drinking now decafe?)
At Fri, 26 Feb 2021 14:33:26 +0900 (JST), Kyotaro Horiguchi
wrote in
> - Tweaked psql to enable tracing. (attached)
>
> - Connect to a server using SSL.
>
> - Restart the server.
->
This is a mistake. Just restaring the server doesn't cause this.
> - Re
(we had better avoid top-posting:p)
At Fri, 26 Feb 2021 02:52:49 +, "iwata@fujitsu.com"
wrote in
> Alvaro san,
>
> Thank you very much for your updating and organizing this patch.
>
> It appears that something is still wrong. I applied lipq pipeline v27 from
> [1] and ran src/test/m
At Fri, 26 Feb 2021 13:35:26 +0900 (JST), Kyotaro Horiguchi
wrote in
> At Thu, 25 Feb 2021 21:04:00 -0300, Álvaro Herrera
> wrote in
> > This assert I added?
> >
> > > +static bool
> > > +pqTraceMaybeBreakLine(int size, PGconn *conn)
> > > +{
> > > + Assert(conn->be_msg->length > 0);
> >
>
At Thu, 25 Feb 2021 21:04:00 -0300, Álvaro Herrera
wrote in
> This assert I added?
>
> > +static bool
> > +pqTraceMaybeBreakLine(int size, PGconn *conn)
> > +{
> > + Assert(conn->be_msg->length > 0);
>
> It breaks. Maybe there's a good reason, maybe there ain't, but in the
> meantime I just
iguchi' ; pgsql-hackers@lists.postgresql.org
Subject: Re: libpq debug log
I tweaked this code a little bit more. I didn't like that libpq-trace.h was
exposing all the internal details of the API to the world; and I liked it even
less that libpq-int.h had to include the file, exposing everything
This assert I added?
> +static bool
> +pqTraceMaybeBreakLine(int size, PGconn *conn)
> +{
> + Assert(conn->be_msg->length > 0);
It breaks. Maybe there's a good reason, maybe there ain't, but in the
meantime I just removed it.
--
Álvaro Herrera Valdivia, Chile
"Para tener más hay que
It appears that something is still wrong. I applied lipq pipeline v27 from [1]
and ran src/test/modules/test_libpq/pipeline singlerow, after patching it to do
PQtrace() after PQconn(). Below is the output I get from that. The noteworthy
point is that "ParseComplete" messages appear multiple tim
I tweaked this code a little bit more. I didn't like that libpq-trace.h was
exposing all the internal details of the API to the world; and I liked it even
less that libpq-int.h had to include the file, exposing everything everywhere.
I added some forward struct declarations in libpq-int.h to avo
Hi Kirk san,
Thank you for your review. I update patch to v21.
> -Original Message-
> From: Jamison, Kirk/ジャミソン カーク
> Sent: Wednesday, February 24, 2021 1:04 PM
> (1) Doc: PQtraceSetFlags
> + flags contains flag bits describing the operating
> mode
> + of tracing. If flags c
> From: alvhe...@alvh.no-ip.org
> I'll give this another look tomorrow, but I wanted to pass along that I prefer
> libpq-trace.{c,h} instead of libpq-logging. I also renamed variable "pin" and
> pgindented. I don't have any major reservations about this patch now, so I'll
> mark it ready-for-com
From: alvhe...@alvh.no-ip.org
> I'll give this another look tomorrow, but I wanted to pass along that I prefer
> libpq-trace.{c,h} instead of libpq-logging. I also renamed variable "pin" and
> pgindented.
Ah, you're right, because the function names are PQtrace() and PQuntrace().
> I don't hav
I'll give this another look tomorrow, but I wanted to pass along that I
prefer libpq-trace.{c,h} instead of libpq-logging. I also renamed
variable "pin" and pgindented. I don't have any major reservations
about this patch now, so I'll mark it ready-for-committer in case
somebody else wants to hav
Alvaro-san, Iwata-san,
From: Iwata, Aya/岩田 彩
> I update patch to v19.
...
> And 3 bugs I noted February 2nd email are all fixed.
> > 1. fix 3 bugs
> > 1.1 -1 output in "Query" message
> > 1.2 two message output in "ReadyForQuery" message
> > 1.3 "StartupMessage" output as " UnknownMessage "
Hi Tsunakawa san,
I update patch to v19.
> -Original Message-
> From: Tsunakawa, Takayuki/綱川 貴之
> Sent: Monday, February 22, 2021 1:30 PM
> (52)
> + of tracing. If (flags contains
> PQTRACE_SUPPRESS_TIMESTAMPS),
>
> () can be removed?
Yes, I removed ().
> (53)
> + int
From: Iwata, Aya/岩田 彩
> I update patch to v18. It has been fixed in response to Tsunakawa san's
> review.
(52)
+ of tracing. If (flags contains
PQTRACE_SUPPRESS_TIMESTAMPS),
() can be removed?
(53)
+ int inLogging; /* next byte of logging
*/
I u
Hi all,
I update patch to v18. It has been fixed in response to Tsunakawa san's review.
> From: Tsunakawa, Takayuki/綱川 貴之
> Sent: Friday, February 12, 2021 1:53 PM
>
> (48)
>
> void PQtrace(PGconn *conn, FILE *stream);
>
>
>
> +
> + Calls PQtraceSetFlags to output with or
(48)
void PQtrace(PGconn *conn, FILE *stream);
+
+ Calls PQtraceSetFlags to output with or without a
timestamp.
+
+
Why is this necessary? Even if you decide to remove this change, can you share
your idea on why you added this just in case?
(49)
+ Dete
Hi all,
Thank you for your review. I update patch to v17.
> From: Tsunakawa, Takayuki/綱川 貴之
> Sent: Tuesday, February 9, 2021 11:26 AM
> (45)
...
> The description of PQtrace() should be written independent of PQtraceEx().
> It is an unnecessary implementation detail to the user that PQtrace()
On 2021-Feb-09, Kyotaro Horiguchi wrote:
> 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);
I like this idea better tha
On 2021-Feb-04, tsunakawa.ta...@fujitsu.com wrote:
> From: 'Alvaro Herrera'
> > > (41)
> > > +void
> > > +PQtraceEx(PGconn *conn, FILE *debug_port, int flags)
> > > +{
> >
> > I'm not really sure about making this a separate API call. We could just
> > make it PQtrace() and increment the libpq s
At Tue, 9 Feb 2021 08:10:19 +, "tsunakawa.ta...@fujitsu.com"
wrote in
> From: Kyotaro Horiguchi
> > > (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);
At Mon, 8 Feb 2021 14:57:47 +, "iwata@fujitsu.com"
wrote in
> I update the patch.
> I modified code according to review comments of Tsunakawa san and Horiguchi
> san.
>
> And I fixed some bugs.
Thanks for the new version.
+typedef enum
+{
+ MSGDIR_FROM_BACKEND,
+ MSGDIR_F
From: Kyotaro Horiguchi
> > (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);
>
> PQtraceSetFlags(conn, 0);
>
I
At Tue, 9 Feb 2021 02:26:25 +, "tsunakawa.ta...@fujitsu.com"
wrote in
> From: Iwata, Aya/岩田 彩
> > I update the patch.
> > I modified code according to review comments of Tsunakawa san and
> > Horiguchi san.
>
>
> I confirmed that all the previous feedback was reflected. Here are some
>
From: Iwata, Aya/岩田 彩
> I update the patch.
> I modified code according to review comments of Tsunakawa san and
> Horiguchi san.
I confirmed that all the previous feedback was reflected. Here are some minor
comments:
(45)
void PQtrace(PGconn *conn, FILE *stream);
+
+ Ca
HI all,
I update the patch.
I modified code according to review comments of Tsunakawa san and Horiguchi san.
And I fixed some bugs.
> This patch should address the following:
> 1. fix 3 bugs
> 1.1 -1 output in "Query" message
The cause of this bug is that it call in pqFlush() function before
From: 'Alvaro Herrera'
> > (41)
> > +void
> > +PQtraceEx(PGconn *conn, FILE *debug_port, int flags)
> > +{
>
> I'm not really sure about making this a separate API call. We could just
> make it PQtrace() and increment the libpq so version. I don't think
> it's a big deal, frankly.
If we change
From: Alvaro Herrera
> I'm pretty sure I named the flag PQTRACE_SUPPRESS_TIMESTAMP (and I
> prefer SUPPRESS to NOOUTPUT), because the idea is that the timestamp is
> printed by default. I think that's the sensible decision: applications
> prefer to have timestamps, even if there's a tiny bit of o
On 2021-Feb-03, tsunakawa.ta...@fujitsu.com wrote:
> From: 'Alvaro Herrera'
> > > + conn->fe_msg->num_fields != DEF_FE_MSGFIELDS)
>
> > The rationale for that second condition is this: if the memory allocated
> > is the initial size, we don't free memory, because it would just be
> > all
On 2021-Feb-03, tsunakawa.ta...@fujitsu.com wrote:
> (39)
> + of tracing. If (flags &
> PQTRACE_OUTPUT_TIMESTAMPS) is
> + true, then timestamp is not printed with each message.
>
> The flag name (OUTPUT) and its description (not printed) doesn't match.
>
> I think you can use less pr
On 2021-Feb-03, Kyotaro Horiguchi wrote:
> Looking the doc mentioned in the comment #39:
>
> + flags contains flag bits describing the operating
> mode
> + of tracing. If (flags &
> PQTRACE_OUTPUT_TIMESTAMPS) is
> + true, then timestamp is not printed with each message.
>
> PQT
At Wed, 3 Feb 2021 01:26:41 +, "tsunakawa.ta...@fujitsu.com"
wrote in
> (41)
> +void
> +PQtraceEx(PGconn *conn, FILE *debug_port, int flags)
> +{
> + if (conn == NULL)
> + return;
> ...
> + if (!debug_port)
> + return;
>
> The if should better be as follows t
From: 'Alvaro Herrera'
> > + conn->fe_msg->num_fields != DEF_FE_MSGFIELDS)
> The rationale for that second condition is this: if the memory allocated
> is the initial size, we don't free memory, because it would just be
> allocated of the same size next time, and that size is not very b
(39)
+ of tracing. If (flags &
PQTRACE_OUTPUT_TIMESTAMPS) is
+ true, then timestamp is not printed with each message.
The flag name (OUTPUT) and its description (not printed) doesn't match.
I think you can use less programmatical expression like "If
flags contains PQTRACE_OUTPUT_TIME
On 2021-Jan-29, tsunakawa.ta...@fujitsu.com wrote:
> (30)
> +/*
> + * Deallocate FE/BE message tracking memory. We only do this because
> + * FE message can grow arbitrarily large, and skip it in the initial state,
> + * because it's likely pointless.
> + */
> +void
> +pqTraceUninit(PGconn *conn)
Hi all,
Thank you Kirk san for creating the v14 patch.
I update the patch. I fixed all of Tsunakawa san's review comments.
I am trying to solve three bugs. Two bags were pointed out by Alvaro san
in a previous e-mail. And I found one bug.
> From: Alvaro Herrera
> Sent: Friday, January 22, 202
From: Jamison, Kirk/ジャミソン カーク
> I realized that existing libpq regression tests in src/test/examples do not
> test PQtrace(). At the same time, no other functions Is calling PQtrace(),
> so it's expected that by default if there are no compilation errors, then it
> will pass all the tests. And it
From: Jamison, Kirk/ジャミソン カーク
> To make the CFBot happy, I fixed the compiler errors.
> I have not included here the change proposal to move the tracing functions to
> a
> new file: fe-trace.c or something, so I retained the changes .
> Maybe Iwata-san can consider the proposal.
> Current
On 2021-Jan-28, k.jami...@fujitsu.com wrote:
> I realized that existing libpq regression tests in src/test/examples do not
> test PQtrace(). At the same time, no other functions Is calling PQtrace(),
> so it's expected that by default if there are no compilation errors, then it
> will pass all the
On Mon, January 25, 2021 4:13 PM (JST), Tsunakawa-san wrote:.
> Also, please note this as:
>
> > Also, why don't you try running the regression tests with a temporary
> modification to PQtrace() to output the trace to a file? The sole purpose is
> to confirm that this patch doesn't make the test
On Mon, Jan 25, 2021 10:11 PM (JST), Alvaro Herrera wrote:
> On 2021-Jan-25, tsunakawa.ta...@fujitsu.com wrote:
>
> > Iwata-san,
>
> [...]
>
> > Considering these and the compilation error Kirk-san found, I'd like
> > you to do more self-review before I resume this review.
>
> Kindly note that
On 2021-Jan-25, tsunakawa.ta...@fujitsu.com wrote:
> Iwata-san,
[...]
> Considering these and the compilation error Kirk-san found, I'd like
> you to do more self-review before I resume this review.
Kindly note that these errors are all mine.
Thanks for the review
--
Álvaro Herrera
Iwata-san,
(25)
+ conn->fe_msg = malloc(sizeof(offsetof(pqFrontendMessage,
fields)) +
+ DEF_FE_MSGFIELDS *
sizeof(pqFrontendMessageField));
It's incorrect that offsetof() is nested in sizeof(). See my original review
comme
Tsunakawa san,
> Strangely, Iwata-san's latest mail she sent today at 10:34 JST hasn't appeared
> on pgsql-hackers yet after more than 6 hours. It is not reflected in the CF
> entry [1]. So, I'm putting her original mail below. The v13 patch attached
> to
> the original mail is attached to th
Hello,
I have not tested nor review the latest patch changes yet, but I am reporting
the compiler errors.
I am trying to compile the patch since V12 (Alvaro's version), but the
following needs to
be fixed too because of the complaints: doc changes and undeclared INT_MAX
libpq.sgml:5893: parser
/岩田 彩
> Sent: Monday, January 25, 2021 10:34 AM
> To: 'Alvaro Herrera' ; Tsunakawa, Takayuki/綱川 貴
> 之
> Cc: 'pgsql-hackers@lists.postgresql.org'
>
> Subject: RE: libpq debug log
>
> Hello, Alvaro san , Tsunakawa san
>
> Thank you for your help. It
Hello, Alvaro san , Tsunakawa san
Thank you for your help. It was very helpful.
> Please integrate Alvaro-san's patch with yours. Next week is the last week
> in this CF, so do your best to post the patch by next Monday or so (before
> Alvaro-san loses interest or time.) Then I'll review it
First, some possibly major questions:
(23)
From: 'Alvaro Herrera'
> Maybe we can create a new file specifically for this to avoid mixing
> unrelated stuff in fe-misc.c -- say fe-trace.c where all this new
> tracing stuff goes.
What do you think about this suggestion? I think this is reasonable
Hello, just two quick comments on this,
On 2021-Jan-22, tsunakawa.ta...@fujitsu.com wrote:
> From: Alvaro Herrera
> > That's true, but it'd require that we move PQtrace() to fe-misc.c, because
> > pqTraceInit() uses definitions that are private to that file.
>
> Ah, that was the reason for sepa
Hello Alvaro-san, Iwata-san,
First of all, thank you Alvaro-san really a lot for your great help. I'm glad
you didn't lose interest and time for this patch yet. (Iwata-san is my
colleague.)
From: Alvaro Herrera
> That's true, but it'd require that we move PQtrace() to fe-misc.c, because
>
On 2021-Jan-17, tsunakawa.ta...@fujitsu.com wrote:
> * I don't see the need for separate pqTraceInit() function, because it is
> only called here.
That's true, but it'd require that we move PQtrace() to fe-misc.c,
because pqTraceInit() uses definitions that are private to that file.
> (2)
> +bo
Hi Iwata-san,
In addition to Tsunakawa-san's comments,
The compiler also complains:
fe-misc.c:678:20: error: ‘lenPos’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
conn->outMsgStart = lenPos;
There's no need for variable lenPos anymore since we have decided *not*
Dear Tsunakawa-san, Iwata-san,
> * Is setlinebuf() available on Windows? Maybe you should use setvbuf()
> instead.
Yeah, cfbot2021 throws errors:
https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.124922
```
src/interfaces/libpq/fe-connect.c(6776): warning C4013: 'setlineb
From: iwata@fujitsu.com
> This patch includes these items:
Is there anything else in this revision that is not handled?
Below are my comments.
Also, why don't you try running the regression tests with a temporary
modification to PQtrace() to output the trace to a file? The sole purpose is
Hi,
Thank you for your review. I modified the code in response to those reviews.
This patch includes these items:
- Fix the code according to reviews
- Fix COPY output issue
- Change to not support Protocol v2.0
It is rarely used anymore and de-support it makes code more simpler.
Plea
Hi Iwata-san
I reviewed v10-0001-libpq-trace.patch. (But I don't check recent discussion...)
I found some bugs.
I'm suggesting some refactoring.
@@ -6809,7 +6809,17 @@ PQtrace(PGconn *conn, FILE *debug_port)
+ if (pqTraceInit(conn))
+ {
+ conn->Pfdebug = debug_port;
Iwata-san,
On Mon, Dec 21, 2020 at 5:20 PM k.jami...@fujitsu.com
wrote:
>
> On Tuesday, December 15, 2020 8:12 PM, Iwata-san wrote:
> > > There are some things still to do:
> > I worked on some to do.
>
> Hi Iwata-san,
>
> Thank you for updating the patch.
> I would recommend to register this pat
From: k.jami...@fujitsu.com
> I understand that protocol 2.0 is still supported, but it is only used for
> PostgreSQL versions 7.3 and earlier, which is not updated by fixes anymore
> since we only backpatch up to previous 5 versions. However I am not sure if
> it's a good idea, but how about if w
On Tuesday, December 15, 2020 8:12 PM, Iwata-san wrote:
> > There are some things still to do:
> I worked on some to do.
Hi Iwata-san,
Thank you for updating the patch.
I would recommend to register this patch in the upcoming commitfest
to help us keep track of it. I will follow the thread to pro
Hi Alvaro san,
> There are some things still to do:
I worked on some to do.
> 1. Is the handling of protocol 2 correct?
Protocol 3.0 is implemented in PostgreSQL v7.4 or later. Therefore, most
servers and clients today want to connect using 3.0.
Also, wishful thinking, I think Protocol 2.0 wil
On Tue, Oct 27, 2020 at 3:23 AM Alvaro Herrera wrote:
>
> 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.
>
Hi,
Hopefully Iwata-san will return to looking at this so
> 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/永浦 良平
> ; nag...@sraoss.co.jp;
> peter.eisentr...@2ndquadrant.com; 'Kyotaro HORIGUCHI'
> ; Jamison, Kirk/ジャミ
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-i
On Wed, Oct 14, 2020 at 10:18:38AM +0900, Kyotaro Horiguchi wrote:
> At Fri, 9 Oct 2020 11:48:59 -0300, Alvaro Herrera
> wrote in
>> Doesn't enlargePQExpBuffer() include room for the trailing zero? I
>> think it does.
>
> Right. I faintly recall I said the same thing before..
FWIW, as pqexpbu
At Fri, 9 Oct 2020 11:48:59 -0300, Alvaro Herrera
wrote in
> > +pqLogMsgString(PGconn *conn, const char *v, int length, PGCommSource
> > commsource)
> > +{
> > + if (length < 0)
> > + length = strlen(v) + 1;
> > +
>
> > pqLogMsgString(conn, str, -1, FROM_*) means actual length may
On Wed, Sep 16, 2020 at 4:41 PM Alvaro Herrera wrote:
> 2020-09-16 13:27:29.072 -03 < ErrorResponse 117 S "ERROR" V "ERROR" C
> "42704" M "no existe el slot de replicación «foobar»" F "slot.c" L "408" R
> "ReplicationSlotAcquireInternal" ^@
Ooh, that looks really nice. The ^@ at the end seems
On 2020-Oct-09, Kyotaro Horiguchi wrote:
> I saw that version and have some comments.
>
> +pqGetProtocolMsgType(unsigned char c, PGCommSource commsource)
> +{
> + const char *message_type;
>
> Compiler complains that this is unused.
>
> +static const char *
> +pqGetProtocolMsgType(unsigned
At Wed, 16 Sep 2020 17:41:55 -0300, Alvaro Herrera
wrote in
> Hello Aya Iwata,
>
> I like this patch, and I think we should have it. I have updated it, as
> it had conflicts.
>
> In 0002, I remove use of ftime(), because that function is obsolete.
> However, with that change we lose printing
resql.org; t...@sss.pgh.pa.us;
> robertmh...@gmail.com; pchamp...@pivotal.io; jd...@pivotal.io;
> raam.s...@gmail.com; Nagaura, Ryohei/永浦 良平
> ; nag...@sraoss.co.jp;
> peter.eisentr...@2ndquadrant.com; 'Kyotaro HORIGUCHI'
> ; Jamison, Kirk/ジャミソン カーク
>
> Subject: Re: libpq d
Hello Aya Iwata,
I like this patch, and I think we should have it. I have updated it, as
it had conflicts.
In 0002, I remove use of ftime(), because that function is obsolete.
However, with that change we lose printing of milliseconds in the trace
lines. I think that's not great, but I also thi
Hello,
Thank you for your review.
I update patch. Please find attached my patch.
> > 2019-04-04 02:39:51.488 UTC > Query 59 "SELECT
> pg_catalog.set_config('search_path', '', false)"
>
> The "59" there seems quite odd, though.
Could you explain more detail about this?
"59" is length of protoc
Hello,
Nice patch. I think there's pretty near consensus that this is
something we want, and that the output format of one trace msg per libpq
msg is roughly okay. (I'm not sure there's 100% agreement on this last
point, but it seems close enough.)
> I changed like this:
>
> 2019-04-04 02:39:5
Hi,
This is a summary of the whole thread.
I am currently improving PQtrace() by adjusting its output to one line per
protocol message as per the advice of reviewers.
Purpose:
If a problem occurs, such as a slow query, you want to know which query takes
time.
In Current libpq, there is PQtrace
Hi,
I rebased my patch from head. Please find my attached patch.
Regard,
Aya Iwata
v5-libpq-PQtrace-output-one-line.patch
Description: v5-libpq-PQtrace-output-one-line.patch
Hi Kirk,
Thank you for your reviewing.
> Docs:
> It would be better to have reference to the documentations of Frontend/Backend
> Protocol's "Message Format".
I added a link to "Protocol's Message Formats" and little explanation to
PQtrace() documentation.
> Code:
> There are some protocol mess
Hi Aya-san,
I tested your v3 patch and it seemed to work on my Linux environment.
However, the CF Patch Tester detected a build failure (probably on Windows).
Refer to: http://commitfest.cputube.org/
Docs:
It would be better to have reference to the documentations of
Frontend/Backend Protocol's
Hi Horiguchi-san,
Thank you for your reviewing.
I updated patch. Please see my attached patch.
> +/* protocol message name */
> +static char *command_text_b[] = {
>
> Couldn't the name be more descriptive? The comment just above doesn't seem
> consistent with the variable. The tables are very
Hello. Thank you for the new patch.
At Tue, 9 Apr 2019 06:19:32 +, "Iwata, Aya"
wrote in <71E660EB361DF14299875B198D4CE5423DF161BA@g01jpexmbkw25>
> Hi,
>
> I update patch to improve PQtrace(); output log message in one line.
> Please find my attached patch.
>
> How it changed:
> > > The ba
Hi,
I update patch to improve PQtrace(); output log message in one line.
Please find my attached patch.
How it changed:
> > The basic idea being:
> >
> > - Each line is a whole message.
> > - The line begins with <<< for a message received and >>> for a message
> sent.
> > - Strings in single quo
Hi,
> The basic idea being:
>
> - Each line is a whole message.
> - The line begins with <<< for a message received and >>> for a message sent.
> - Strings in single quotes are those sent/received as a fixed number of bytes.
> - Strings in double quotes are those sent/received as a string.
> - 4
On 3/5/19 5:28 PM, David Steele wrote:
On 3/5/19 11:48 AM, Iwata, Aya wrote:
So is it alright to add these information to the new/proposed
PQtrace() default output?
I agree with Andres [1] that it's not very clear where this patch is
going and we should push the target to PG13.
Hearing no
On 3/5/19 11:48 AM, Iwata, Aya wrote:
So is it alright to add these information to the new/proposed PQtrace() default
output?
I agree with Andres [1] that it's not very clear where this patch is
going and we should push the target to PG13.
Regards,
--
-David
da...@pgmasters.net
[1]
htt
Hi everyone,
I appreciate all the helpful advice.
I agree to display message more clearly. I will follow your advice.
I would like to add timestamp per line and when command processing function
start/end.
I think it is useful to know the application process start/end for diagnosis.
So I
Hi Horiguchi-san,
Thank you for your reply and suggestions.
> > 1. Expand PQtrace() facility and improve libpq logging.
> >
> > 2. Prepare "output level". There are 3 type of levels;
> > - TRADITIONAL : if set, outputs protocol messages
> > - LEVEL1: if set, outputs ph
On Mon, Mar 4, 2019 at 10:25 PM Tom Lane wrote:
> Robert Haas writes:
> > The basic idea being:
> > - Each line is a whole message.
> > - The line begins with <<< for a message received and >>> for a message
> > sent.
>
> +1, though do we really need to repeat the direction marker thrice?
Perha
Robert Haas writes:
> The basic idea being:
> - Each line is a whole message.
> - The line begins with <<< for a message received and >>> for a message sent.
+1, though do we really need to repeat the direction marker thrice?
> - Strings in single quotes are those sent/received as a fixed numbe
On Mon, Mar 4, 2019 at 3:13 AM Iwata, Aya wrote:
> 2. Prepare "output level". There are 3 type of levels;
> - TRADITIONAL : if set, outputs protocol messages
> - LEVEL1: if set, outputs phase and time
> - LEVEL2: if set, outputs both inf
Hello.
I came up with some random comments.
At Mon, 4 Mar 2019 08:13:00 +, "Iwata, Aya"
wrote in <71E660EB361DF14299875B198D4CE5423DEF1844@g01jpexmbkw25>
> Hi,
>
> Thank you for your comments and advice.
>
> I'd like to consider your suggestions.
> I am planning to change libpq logging li
Hi,
Thank you for your comments and advice.
I'd like to consider your suggestions.
I am planning to change libpq logging like this;
1. Expand PQtrace() facility and improve libpq logging.
2. Prepare "output level". There are 3 type of levels;
- TRADITIONAL : if set, outputs protocol
Robert Haas writes:
> On Thu, Feb 21, 2019 at 7:52 PM Iwata, Aya wrote:
>> Aside from problems with my current documentation which I will fix,
>> could you explain more detail about the problem of the design?
> We already have a PQtrace() facility that could be improved, and it
> has been previo
On Thu, Feb 21, 2019 at 7:52 PM Iwata, Aya wrote:
> > I'm not really sure that I like the design of this patch in any way.
> Aside from problems with my current documentation which I will fix,
> could you explain more detail about the problem of the design?
> I would like to improve my current imp
On Wednesday, February 20, 2019 12:56 PM GMT+9, Robert Haas wrote:
> On Mon, Feb 18, 2019 at 10:06 PM Jamison, Kirk
> wrote:
> > It sounds more logical to me if there is a parameter that switches
> > on/off the logging similar to other postgres logs. I suggest trace_log as
> > the parameter na
Hi Robert,
> I'm not really sure that I like the design of this patch in any way.
Aside from problems with my current documentation which I will fix,
could you explain more detail about the problem of the design?
I would like to improve my current implementation based from feedback.
Regards,
Ay
Hi Kirk,
> Currently, the patch fails to build according to CF app.
> As you know, it has something to do with the misspelling of function.
> GetTimezoneInformation --> GetTimeZoneInformation
Thank you. I fixed it. Please see my v7 patch.
Regards,
Aya Iwata
Hi Ramanarayana,
Thank you for your review and suggestion. Please see the attached updated patch.
> Issues found while testing
>1) If invalid value is given to PGLOGMINLEVEL, empty log file is created which
>should not happen.
Thank you for your test. However in my dev environment, empty log fil
101 - 200 of 232 matches
Mail list logo
