On Mon, Jun 07, 2021 at 11:37:58AM -0400, Robert Haas wrote:
> On Sat, Jun 5, 2021 at 11:03 AM Alvaro Herrera
> wrote:
> > > This added a PQtraceSetFlags() function. We have a dozen PQset*()
> > > functions,
> > > but this and PQresultSetInstanceData() are the only PQSomethingSet()
> > >
On Sat, Jun 5, 2021 at 11:03 AM Alvaro Herrera wrote:
> > This added a PQtraceSetFlags() function. We have a dozen PQset*()
> > functions,
> > but this and PQresultSetInstanceData() are the only PQSomethingSet()
> > functions. Is it okay if I rename it to PQsetTraceFlags()? I think that
> >
On 2021-Jun-04, Noah Misch wrote:
> On Fri, Apr 09, 2021 at 05:16:11PM -0400, Alvaro Herrera wrote:
> > Pushed now.
>
> This added a PQtraceSetFlags() function. We have a dozen PQset*() functions,
> but this and PQresultSetInstanceData() are the only PQSomethingSet()
> functions. Is it okay if
On Fri, Apr 09, 2021 at 05:16:11PM -0400, Alvaro Herrera wrote:
> Pushed now.
This added a PQtraceSetFlags() function. We have a dozen PQset*() functions,
but this and PQresultSetInstanceData() are the only PQSomethingSet()
functions. Is it okay if I rename it to PQsetTraceFlags()? I think
On 2021-Apr-02, Tom Lane wrote:
> Alvaro Herrera writes:
> > On 2021-Apr-02, Tom Lane wrote:
> >> +1, but the comment could be more specific. Maybe like "In regress mode,
> >> suppress the length of ErrorResponse and NoticeResponse messages --- the F
> >> (file name) field, in particular, can
Alvaro Herrera writes:
> On 2021-Apr-02, Tom Lane wrote:
>> +1, but the comment could be more specific. Maybe like "In regress mode,
>> suppress the length of ErrorResponse and NoticeResponse messages --- the F
>> (file name) field, in particular, can vary in length depending on compiler
>>
On 2021-Apr-02, Tom Lane wrote:
> Alvaro Herrera writes:
> > As in the attached patch.
>
> +1, but the comment could be more specific. Maybe like "In regress mode,
> suppress the length of ErrorResponse and NoticeResponse messages --- the F
> (file name) field, in particular, can vary in
On 2021-Apr-02, Tom Lane wrote:
> On third thought, maybe we should push your patch too. Although I think
> 53aafdb9f is going to fix the platform-specific aspect of this, we are
> still going to risk some implementation dependence of the libpq_pipeline
> results:
>
> * Every so often, the
I wrote:
>> I bet what is happening on drongo is that the compiler has generated a
>> __FILE__ value that contains backslashes not slashes, and this code
>> doesn't know how to shorten those. So maybe instead of lobotomizing
>> this test, we should fix that.
> Did that, but we'll have to wait a
On 2021-Apr-02, Tom Lane wrote:
> I wrote:
> > I bet what is happening on drongo is that the compiler has generated a
> > __FILE__ value that contains backslashes not slashes, and this code
> > doesn't know how to shorten those. So maybe instead of lobotomizing
> > this test, we should fix that.
I wrote:
> I bet what is happening on drongo is that the compiler has generated a
> __FILE__ value that contains backslashes not slashes, and this code
> doesn't know how to shorten those. So maybe instead of lobotomizing
> this test, we should fix that.
Did that, but we'll have to wait a few
Alvaro Herrera writes:
> As in the attached patch.
+1, but the comment could be more specific. Maybe like "In regress mode,
suppress the length of ErrorResponse and NoticeResponse messages --- the F
(file name) field, in particular, can vary in length depending on compiler
used."
Actually
On 2021-Apr-02, Alvaro Herrera wrote:
> On 2021-Apr-02, Kyotaro Horiguchi wrote:
>
> > At Fri, 02 Apr 2021 14:40:09 +0900 (JST), Kyotaro Horiguchi
> > wrote in
>
> > > So, the cheapest measure for regression test would be just making the
> >
> > So, the cheapest measure for regression test
On 2021-Apr-02, Kyotaro Horiguchi wrote:
> At Fri, 02 Apr 2021 14:40:09 +0900 (JST), Kyotaro Horiguchi
> wrote in
> > So, the cheapest measure for regression test would be just making the
>
> So, the cheapest measure for regression test would be just *masking* the
>
> > length field, while
At Fri, 02 Apr 2021 01:45:06 -0400, Tom Lane wrote in
> Kyotaro Horiguchi writes:
> > At Fri, 2 Apr 2021 02:56:44 +, "iwata@fujitsu.com"
> > wrote in
> >> Do ErrorResponse and NoticeResponse vary from test to test ...?
> >> If so, it seemed difficult to make the trace logs available
At Fri, 02 Apr 2021 14:40:09 +0900 (JST), Kyotaro Horiguchi
wrote in
> At Fri, 2 Apr 2021 02:56:44 +, "iwata@fujitsu.com"
> wrote in
> > Hi Alvaro san
> >
> > Thank you for your fix of trace log code.
> >
> > > From: 'alvhe...@alvh.no-ip.org'
> > > Sent: Friday, April 2, 2021
Kyotaro Horiguchi writes:
> At Fri, 2 Apr 2021 02:56:44 +, "iwata@fujitsu.com"
> wrote in
>> Do ErrorResponse and NoticeResponse vary from test to test ...?
>> If so, it seemed difficult to make the trace logs available for regression
>> test.
>> I will also investigate the cause of
At Fri, 2 Apr 2021 02:56:44 +, "iwata@fujitsu.com"
wrote in
> Hi Alvaro san
>
> Thank you for your fix of trace log code.
>
> > From: 'alvhe...@alvh.no-ip.org'
> > Sent: Friday, April 2, 2021 11:30 AM
> ...
> > It still didn't fix it! Drongo is now reporting a difference in the
> >
Hi Alvaro san
Thank you for your fix of trace log code.
> From: 'alvhe...@alvh.no-ip.org'
> Sent: Friday, April 2, 2021 11:30 AM
...
> It still didn't fix it! Drongo is now reporting a difference in the expected
> trace --
> and the differences all seem to be message lengths.
> Now that is
On 2021-Apr-01, 'alvhe...@alvh.no-ip.org' wrote:
> Ooh, wow ... now that is a silly bug! Thanks, I'll push the fix in a
> minute.
It still didn't fix it! Drongo is now reporting a difference in the
expected trace -- and the differences all seem to be message lengths.
Now that is pretty
On 2021-Apr-01, Tom Lane wrote:
> So drongo is still failing, and after a bit of looking around at
> other code I finally got hit with the clue hammer. Per port.h:
>
> * On Windows, setvbuf() does not support _IOLBF mode, and interprets that
> * as _IOFBF. To add insult to injury,
On 2021-Apr-01, Tom Lane wrote:
> "'alvhe...@alvh.no-ip.org'" writes:
> > On 2021-Apr-01, Tom Lane wrote:
> >> BTW, what in the world is this supposed to accomplish?
> >> -(long long) rows_to_send);
> >> +(1L << 62) + (long long) rows_to_send);
>
So drongo is still failing, and after a bit of looking around at
other code I finally got hit with the clue hammer. Per port.h:
* On Windows, setvbuf() does not support _IOLBF mode, and interprets that
* as _IOFBF. To add insult to injury, setvbuf(file, NULL, _IOFBF, 0)
* crashes outright if
"'alvhe...@alvh.no-ip.org'" writes:
> On 2021-Apr-01, Tom Lane wrote:
>> BTW, what in the world is this supposed to accomplish?
>> -(long long) rows_to_send);
>> +(1L << 62) + (long long) rows_to_send);
> It makes the text representation wider,
On 2021-Apr-01, Tom Lane wrote:
> BTW, what in the world is this supposed to accomplish?
>
> -(long long) rows_to_send);
> +(1L << 62) + (long long) rows_to_send);
>
> Various buildfarm members are complaining that the shift distance
> is more
BTW, what in the world is this supposed to accomplish?
-(long long) rows_to_send);
+(1L << 62) + (long long) rows_to_send);
Various buildfarm members are complaining that the shift distance
is more than the width of "long", which I'd just fix with
"'alvhe...@alvh.no-ip.org'" writes:
> Eh, so I forgot to strdup(optarg[optind]). Apparently that works fine
> in glibc but other getopt implementations are likely not so friendly.
Hmm ... interesting theory, but I don't think I buy it, since the
program isn't doing anything that should damage
On 2021-Mar-31, Tom Lane wrote:
> I wrote:
> > That is weird - only test 4 (of 8) runs at all, the rest seem to
> > fail to connect. What's different about pipelined_insert?
>
> Oh ... there's a pretty obvious theory. pipelined_insert is
> the only one that is not asked to write a trace file.
On 2021-Mar-31, Tom Lane wrote:
> While this may have little to do with drongo's issue,
> I'm going to take exception to this bit that I see that
> the patch added to PQtrace():
>
> /* Make the trace stream line-buffered */
> setvbuf(debug_port, NULL, _IOLBF, 0);
Mea culpa. I added
I wrote:
> What I suspect is some Windows dependency in the way that
> 001_libpq_pipeline.pl is setting up the trace output files.
While this may have little to do with drongo's issue,
I'm going to take exception to this bit that I see that
the patch added to PQtrace():
/* Make the trace
"'alvhe...@alvh.no-ip.org'" writes:
> On 2021-Mar-31, Tom Lane wrote:
>> So for some reason, opening the trace file fails.
>> (I wonder why we don't see an error message for this though.)
> .. oh, I think we forgot to set conn->Pfdebug = NULL when creating the
> connection. So when we do
On 2021-Mar-31, 'alvhe...@alvh.no-ip.org' wrote:
> .. oh, I think we forgot to set conn->Pfdebug = NULL when creating the
> connection. So when we do PQtrace(), the first thing it does is
> PQuntrace(), and then that tries to do fflush(conn->Pfdebug) ---> crash.
> So this should fix it.
I tried
On 2021-Mar-31, Tom Lane wrote:
> I wrote:
> > That is weird - only test 4 (of 8) runs at all, the rest seem to
> > fail to connect. What's different about pipelined_insert?
>
> Oh ... there's a pretty obvious theory. pipelined_insert is
> the only one that is not asked to write a trace file.
I wrote:
> That is weird - only test 4 (of 8) runs at all, the rest seem to
> fail to connect. What's different about pipelined_insert?
Oh ... there's a pretty obvious theory. pipelined_insert is
the only one that is not asked to write a trace file.
So for some reason, opening the trace file
"'alvhe...@alvh.no-ip.org'" writes:
> This is not the *only* issue though; at least animal drongo shows a
> completely different failure, where the last few tests don't even get to
> run, and the server log just has this:
That is weird - only test 4 (of 8) runs at all, the rest seem to
fail to
On 2021-Mar-31, Tom Lane wrote:
> I think this is a timing problem that's triggered (on some machines)
> by force_parallel_mode = regress. Looking at spurfowl's latest
> failure of this type, the postmaster log shows
>
> 2021-03-31 14:34:54.982 EDT [18233:15] 001_libpq_pipeline.pl LOG: execute
"'alvhe...@alvh.no-ip.org'" writes:
> So crake failed. The failure is that it doesn't print the DataRow
> messages. That's quite odd. We see this in the trace log:
I think this is a timing problem that's triggered (on some machines)
by force_parallel_mode = regress. Looking at spurfowl's
From: 'alvhe...@alvh.no-ip.org'
> > got expected ERROR: cannot insert multiple commands into a prepared
> statement
> > got expected division-by-zero
>
> Eh? this is not at all expected, of course, but clearly not PQtrace's
> fault. I'll look tomorrow.
Iwata-san and I were starting to
On 2021-Mar-30, 'alvhe...@alvh.no-ip.org' wrote:
> got expected ERROR: cannot insert multiple commands into a prepared statement
> got expected division-by-zero
Eh? this is not at all expected, of course, but clearly not PQtrace's
fault. I'll look tomorrow.
--
Álvaro Herrera
From: 'alvhe...@alvh.no-ip.org'
> Okay, pushed this patch and the new testing for it based on
> libpq_pipeline. We'll see how the buildfarm likes it.
Thank you very much! I appreciate you taking your valuable time while I
imagine you must be pretty busy with taking care of other (possibly
So crake failed. The failure is that it doesn't print the DataRow
messages. That's quite odd. We see this in the trace log:
Mar 30 20:05:15 # F 54 Parse"" "SELECT 1.0/g FROM
generate_series(3, -1, -1) g" 0
Mar 30 20:05:15 # F 12 Bind "" "" 0 0 0
Mar 30 20:05:15 # F
Okay, pushed this patch and the new testing for it based on
libpq_pipeline. We'll see how the buildfarm likes it.
I made some further changes to the last version; user-visibly, I split
the trace flags in two, keeping the timestamp suppression separate from
the redacting feature for regression
Hi Alvaro san, Tsunakawa san
Thank you for creating the v30 patch.
> From: Tsunakawa, Takayuki/綱川 貴之
> Sent: Monday, March 29, 2021 9:45 AM
...
> Iwata-san,
> Please review Alvaro-san's code, and I think you can integrate all patches
> into
> one except for 0002 and 0007. Those two patches
From: 'alvhe...@alvh.no-ip.org'
> > > (Hey, what the heck is that "Z" at the end of the message? I thought
> > > the error ended right at the \x00 ...)
> >
> > We'll investigate these issues.
>
> For what it's worth, I did fix this problem in patch 0005 that I
> attached. The problem was that
From: Kyotaro Horiguchi
> It's better to be short as far as it is clear enough. Actually '<' to
> 'F' and '>' to 'B' is clear enough to me. So I don't need a longer
> notation.
Agreed, because the message format description in the PG manual uses F and B.
Regards
Takayuki Tsunakawa
At Mon, 29 Mar 2021 00:02:58 -0300, "'alvhe...@alvh.no-ip.org'"
wrote in
> On 2021-Mar-29, tsunakawa.ta...@fujitsu.com wrote:
>
> > > (Hey, what the heck is that "Z" at the end of the message? I thought
> > > the error ended right at the \x00 ...)
> >
> > We'll investigate these issues.
>
>
On 2021-Mar-29, tsunakawa.ta...@fujitsu.com wrote:
> > (Hey, what the heck is that "Z" at the end of the message? I thought
> > the error ended right at the \x00 ...)
>
> We'll investigate these issues.
For what it's worth, I did fix this problem in patch 0005 that I
attached. The problem was
From: alvhe...@alvh.no-ip.org
> > Proposed changes on top of v29.
>
> This last one uses libpq_pipeline -t and verifies the output against an
> expected
> trace file. Applies on top of all the previous patches. I attach the whole
> lot,
> so that the CF bot has a chance to run it.
Thank you
From: alvhe...@alvh.no-ip.org
> I added an option to the new libpq_pipeline program that it activates
> libpq trace. It works nicely and I think we can add that to the
> regression tests.
That's good news. Thank you.
> 1. The trace output for the error message won't be very nice, because it
On 2021-Mar-27, alvhe...@alvh.no-ip.org wrote:
> This last one uses libpq_pipeline -t and verifies the output against an
> expected trace file. Applies on top of all the previous patches. I
> attach the whole lot, so that the CF bot has a chance to run it.
All tests pass, but CFbot does not
On 2021-Mar-26, alvhe...@alvh.no-ip.org wrote:
> Proposed changes on top of v29.
This last one uses libpq_pipeline -t and verifies the output against an
expected trace file. Applies on top of all the previous patches. I
attach the whole lot, so that the CF bot has a chance to run it.
I did
Proposed changes on top of v29.
--
Álvaro Herrera Valdivia, Chile
>From b32ae3805bb28553c0a1cf308c6ed27f58576f3c Mon Sep 17 00:00:00 2001
From: Alvaro Herrera
Date: Fri, 26 Mar 2021 19:12:12 -0300
Subject: [PATCH 1/5] libpq_pipeline: add -t support for PQtrace
---
Hello
I added an option to the new libpq_pipeline program that it activates
libpq trace. It works nicely and I think we can add that to the
regression tests. However I have two observations.
1. The trace output for the error message won't be very nice, because it
prints line numbers. So if I
Alvaro-san, Iwata-san,
cc: Tom-san, Horiguchi-san,
Thank you, it finally looks complete to me!
Alvaro-san,
We appreciate your help and patience, sometimes rewriting large part of the
patch. Could you do the final check (and possibly tweak) for commit?
Regards
Takayuki Tsunakawa
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
Hello Iwata-san,
Thanks to removing the static arrays, the code got much smaller. I'm happy
with this result.
(1)
+ ( for messages from client to server,
+ and for messages from server to client),
I think this was meant to say "> or <". So, maybe you should remove "," at the
Hi Horiguchi san and Tsunakawa san,
Thank you for you review.
I update patch to v28. In this patch, I removed array.
And I fixed some code according to Horiguchi san and Tsunakawa san review
comment.
> From: Tsunakawa, Takayuki/綱川 貴之
> Sent: Thursday, March 18, 2021 12:38 PM
> I sort of
From: Kyotaro Horiguchi
> + pqTraceOutputR(const char *message, FILE *f)
> + {
> + int cursor = 0;
> +
> + pqTraceOutputInt32(message, , f);
>
> I don't understand the reason for spliting message and here.
>
> + pqTraceOutputR(const char *message, FILE *f)
> + {
> + char *p =
At Thu, 18 Mar 2021 07:34:36 +, "tsunakawa.ta...@fujitsu.com"
wrote in
> From: Iwata, Aya/岩田 彩
> > > Yes, precisely, 2 bytes for the double quotes needs to be subtracted
> > > as
> > > follows:
> > >
> > > len = fprintf(...);
> > > *cursor += (len - 2);
> >
> > Thank you for your
From: Iwata, Aya/岩田 彩
> > Yes, precisely, 2 bytes for the double quotes needs to be subtracted
> > as
> > follows:
> >
> > len = fprintf(...);
> > *cursor += (len - 2);
>
> Thank you for your advice. I changed pqTraceOutputString set cursor to fprintf
> return -2.
> And I removed cursor
Hi Alvaro san and Tsunakawa san,
Thank you for your review. I updated patch to v27.
`make check` output is following. I think it is OK.
```
2021-03-18 07:02:55.090598 < ReadyForQuery 5 I
2021-03-18 07:02:55.090672 > Terminate 4
From: Alvaro Herrera
> In pqTraceOutputString(), you can use the return value from fprintf to
> move the cursor -- no need to count chars.
Yes, precisely, 2 bytes for the double quotes needs to be subtracted as follows:
len = fprintf(...);
*cursor += (len - 2);
> I still think
I'll look at the comments from Alvaro-san and Horiguchi-san. Here are my
review comments:
(23)
+ /* Trace message only when there is first 1 byte */
+ if (conn->Pfdebug && conn->outCount < conn->outMsgStart)
+ {
+ if (conn->outCount < conn->outMsgStart)
+
Hello
In pqTraceOutputString(), you can use the return value from fprintf to
move the cursor -- no need to count chars.
I still think that the message-type specific functions should print the
message type, rather than having the string arrays.
--
Álvaro Herrera Valdivia, Chile
"La gente
Hi Tsunakawa san, Alvaro san,
Thank you very much for your review. It helped me a lot to make the patch
better.
I update patch to v26.
This patch has been updated according to Tsunakawa san and Alvaro san review
comments.
The output is following;
```
2021-03-17 14:43:16.411238 > Terminate
At Wed, 17 Mar 2021 02:09:32 +, "tsunakawa.ta...@fujitsu.com"
wrote in
> Alvaro-san,
>
>
> Thank you for taking your time to take a look at an incomplete patch. I
> thought I would ask you for final check for commit after Iwata-san has
> reflected my review comments.
>
> I discussed
Alvaro-san,
Thank you for taking your time to take a look at an incomplete patch. I
thought I would ask you for final check for commit after Iwata-san has
reflected my review comments.
I discussed with Iwata-sn your below comments. Let me convey her opinions.
(She is now focusing on
On 2021-Mar-15, iwata@fujitsu.com wrote:
> I create protocol message reading function for each protocol message type.
> (Ex. pqTraceOutputB() read Bind message)
> This makes the nesting shallower and makes the code easier to read.
I'm not sure I agree with this structural change. Yes, it
I've finished the review. Here are the last set of comments.
(16)
(15) is also true for the processing of 'H' message.
(17) pqTraceOutputD
+ for (i = 0; i < nfields; i++)
+ {
+ len = pqTraceOutputInt32(message + cursor, f);
+
I've not finished reviewing yet, but there seems to be many mistakes. I'm
sending second set of review comments now so you can fix them in parallel.
(8)
+ charid = '\0';
This initialization is not required because id will always be assigned a value
shortly.
(9)
+static
I'm looking at the last file libpq-trace.c. I'll continue the review after
lunch. Below are some comments so far.
(1)
- Enables tracing of the client/server communication to a debugging file
stream.
+ Enables tracing of the client/server communication to a debugging file
+
Alvaro san, Tom san Horiguchi san, Tsunakawa san and Kirk san,
Thank you very much for review and advice.
> > Works for me.
>
> Pushed that. I think we're now waiting on Iwata-san to finish a new version
> of
> the tracing patch.
Thank you very much Alvaro san and Tom san. Your patch and code
Alvaro Herrera writes:
> On 2021-Mar-10, Tom Lane wrote:
>> After studying this further, I think we should apply the attached
>> patch to remove that responsibility from pqParseInput3's subroutines.
>> This will allow a single trace call near the bottom of pqParseInput3
>> to handle all cases
At Thu, 11 Mar 2021 04:12:57 +, "tsunakawa.ta...@fujitsu.com"
wrote in
> From: Kyotaro Horiguchi
> > Right. So something like this?
> >
> > unsigned char p;
> >
> > p = buf + *cursor;
> > result = (uint32) (*p << 24) + (*(p + 1)) << 16 + ...);
>
> Yes, that would work (if p is a
From: Kyotaro Horiguchi
> Right. So something like this?
>
> unsigned char p;
>
> p = buf + *cursor;
> result = (uint32) (*p << 24) + (*(p + 1)) << 16 + ...);
Yes, that would work (if p is a pointer), but I think memcpy() is enough like
pqGetInt() does.
Regards
Takayuki Tsunakawa
At Thu, 11 Mar 2021 03:01:16 +, "tsunakawa.ta...@fujitsu.com"
wrote in
> From: Kyotaro Horiguchi
> > The output functions copy message bytes into local variable but the
> > same effect can be obtained by just casing via typed pointer type.
> >
> > uint32 tmp4;
> > ..
> > memcpy(,
At Thu, 11 Mar 2021 01:51:55 +, "tsunakawa.ta...@fujitsu.com"
wrote in
> Alvaro-san, Tom-san, Horiguchi-san,
>
> From: Kyotaro Horiguchi
> > +1 for the thanks for the quick work. I have some random comments
> > after a quick look on it.
>
> Thank you very much for giving many comments.
At Wed, 10 Mar 2021 20:38:20 -0500, Tom Lane wrote in
> Kyotaro Horiguchi writes:
> > Maybe I'm missing something, but the above doesn't seem working. I
> > didn't see such message when making a SSL connection.
>
> As far as that goes, I don't see any point in trying to trace
>
From: Kyotaro Horiguchi
> The output functions copy message bytes into local variable but the
> same effect can be obtained by just casing via typed pointer type.
>
> uint32 tmp4;
> ..
> memcpy(, buf + *cursor, 4);
> result = (int) pg_ntoh32(tmp4);
>
> can be written as
>
> result =
Alvaro-san, Tom-san, Horiguchi-san,
From: Kyotaro Horiguchi
> +1 for the thanks for the quick work. I have some random comments
> after a quick look on it.
Thank you very much for giving many comments. And We're sorry to have caused
you trouble. I told Iwata-san yesterday to modify the
Kyotaro Horiguchi writes:
> Maybe I'm missing something, but the above doesn't seem working. I
> didn't see such message when making a SSL connection.
As far as that goes, I don't see any point in trying to trace
connection-related messages, because there is no way to enable
tracing on a
At Wed, 10 Mar 2021 10:31:27 -0300, "'alvhe...@alvh.no-ip.org'"
wrote in
> On 2021-Mar-10, iwata@fujitsu.com wrote:
>
> > Hi all,
> >
> > Following all reviewer's advice, I have created a new patch.
> >
> > In this patch, I add only two tracing entry points; I call
> >
Alvaro Herrera writes:
> On 2021-Mar-10, Tom Lane wrote:
>> After studying this further, I think we should apply the attached
>> patch to remove that responsibility from pqParseInput3's subroutines.
>> This will allow a single trace call near the bottom of pqParseInput3
>> to handle all cases
On 2021-Mar-10, Tom Lane wrote:
> After studying this further, I think we should apply the attached
> patch to remove that responsibility from pqParseInput3's subroutines.
> This will allow a single trace call near the bottom of pqParseInput3
> to handle all cases that that function processes.
I wrote:
> Or we could rethink the logic. It's not quite clear to me, after
> all this time, why getRowDescriptions() et al are allowed to
> move inStart themselves rather than letting the main loop in
> pqParseInput3 do it. It might well be an artifact of having not
> rewritten the v2 logic too
On 2021-Mar-10, Tom Lane wrote:
> Or we could rethink the logic. It's not quite clear to me, after
> all this time, why getRowDescriptions() et al are allowed to
> move inStart themselves rather than letting the main loop in
> pqParseInput3 do it. It might well be an artifact of having not
>
On 2021-Mar-10, Tom Lane wrote:
> "'alvhe...@alvh.no-ip.org'" writes:
> > After staring at it a couple of times, I think that the places in
> > pqParseInput3() where there's a comment "... moves inStart itself" and
> > then "continue;" should have a call to pqTraceOutputMsg(), since AFAIU
> >
"'alvhe...@alvh.no-ip.org'" writes:
> After staring at it a couple of times, I think that the places in
> pqParseInput3() where there's a comment "... moves inStart itself" and
> then "continue;" should have a call to pqTraceOutputMsg(), since AFAIU
> those are the places where the message in
"'alvhe...@alvh.no-ip.org'" writes:
> After staring at it a couple of times, I think that the places in
> pqParseInput3() where there's a comment "... moves inStart itself" and
> then "continue;" should have a call to pqTraceOutputMsg(), since AFAIU
> those are the places where the message in
On 2021-Mar-10, 'alvhe...@alvh.no-ip.org' wrote:
> I also found that DataRow messages are not being printed. This seems to
> fix that in the normal case and singlerow, at least in pipeline mode.
> Didn't verify the non-pipeline mode.
Hm, and RowDescription ('T') messages are also not being
I also found that DataRow messages are not being printed. This seems to
fix that in the normal case and singlerow, at least in pipeline mode.
Didn't verify the non-pipeline mode.
--
Álvaro Herrera39°49'30"S 73°17'W
"Nunca se desea ardientemente lo que solo se desea
On 2021-Mar-10, iwata@fujitsu.com wrote:
> Hi all,
>
> Following all reviewer's advice, I have created a new patch.
>
> In this patch, I add only two tracing entry points; I call
> pqTraceOutputMsg(PGconn conn, int msgCursor, PGCommSource commsource) in
> pqParseInput3 () and pqPutMsgEnd
h 5, 2021 10:41 PM
> To: Tsunakawa, Takayuki/綱川 貴之
> Cc: 'Tom Lane' ; Iwata, Aya/岩田 彩
> ; Jamison, Kirk/ジャミソン カーク
> ; 'Kyotaro Horiguchi' ;
> pgsql-hackers@lists.postgresql.org
> Subject: Re: libpq debug log
>
> On 2021-Mar-05, tsunakawa.ta...@fujitsu.com wrote:
>
> > From: To
On 2021-Mar-05, tsunakawa.ta...@fujitsu.com wrote:
> From: Tom Lane
> > But I think passing the message start address explicitly might be
> > better than having it understand the buffering behavior in enough
> > detail to know where to find the message. Part of the point here
> > (IMO) is to
Hi Alvaro-san and Horiguchi-san,
CC) Iwata-san, Tsunakawa-san
Attached is the V23 of the libpq trace patch.
(1)
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
From: Tom Lane
> But I think passing the message start address explicitly might be better than
> having it understand the buffering behavior in enough detail to know where to
> find the message. Part of the point here
> (IMO) is to decouple the tracing logic from the core libpq logic, in hopes
"tsunakawa.ta...@fujitsu.com" writes:
> Oops, the logging facility needs the destination (conn->pfdebug). So, it
> will be:
> void pqLogMessage(PGconn *conn, bool toServer);
Right, and it'd need the debug flags too, so +1 for just passing the
PGconn instead of handing those over
From: tsunakawa.ta...@fujitsu.com
> I understood that the former is pqParseInput3() and the latter is
> pqPutMsgEnd(). They call the logging function wne conn->pfdebug is not
> NULL. Its signature is like this (that will be defined in libpq-trace.c):
>
> void pqLogMessage(const char
Tom-san, Alvaro-san,
From: Tom Lane
> I took a quick look through the v22 patch, and TBH I don't like much of
> anything
> at all about the proposed architecture. It's retained most of the flavor of
> the
> way it was done before, which was a hangover from the old process-on-the-fly
>
"tsunakawa.ta...@fujitsu.com" writes:
> 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
1 - 100 of 230 matches
Mail list logo
