On Tue, Nov 7, 2023 at 3:56 AM David Rowley wrote:
>
> On Thu, 2 Nov 2023 at 22:42, Amit Kapila wrote:
> > The other two look good to me.
>
> Thanks for looking.
>
> I spent some time trying to see if the performance changes much with
> either of these cases. For the XLogWalRcvProcessMsg() I was
On Thu, 2 Nov 2023 at 22:42, Amit Kapila wrote:
> The other two look good to me.
Thanks for looking.
I spent some time trying to see if the performance changes much with
either of these cases. For the XLogWalRcvProcessMsg() I was unable to
measure any difference even when replaying inserts into
On Tue, Oct 31, 2023 at 2:25 AM David Rowley wrote:
>
> On Mon, 30 Oct 2023 at 23:48, Amit Kapila wrote:
> >
> > On Fri, Oct 27, 2023 at 3:23 AM David Rowley wrote:
> > > * parallel.c in HandleParallelMessages():
> > > * applyparallelworker.c in HandleParallelApplyMessages():
> >
> > Both the ab
On Mon, 30 Oct 2023 at 23:48, Amit Kapila wrote:
>
> On Fri, Oct 27, 2023 at 3:23 AM David Rowley wrote:
> > * parallel.c in HandleParallelMessages():
> > * applyparallelworker.c in HandleParallelApplyMessages():
>
> Both the above calls are used to handle ERROR/NOTICE messages from
> parallel wo
On Fri, Oct 27, 2023 at 3:23 AM David Rowley wrote:
>
> On Thu, 26 Oct 2023 at 17:00, David Rowley wrote:
> > Thanks for looking at this again. I fixed up each of those and pushed
> > the result, mentioning the incompatibility in the commit message.
> >
> > Now that that's done, I've attached a p
On Thu, 26 Oct 2023 at 17:00, David Rowley wrote:
> Thanks for looking at this again. I fixed up each of those and pushed
> the result, mentioning the incompatibility in the commit message.
>
> Now that that's done, I've attached a patch which makes use of the new
> initReadOnlyStringInfo initiali
On Thu, 26 Oct 2023 at 08:43, Tom Lane wrote:
> I think that we can make that assumption starting with v17.
> Back-patching it would be hazardous perhaps; but if there's some
> function out there that depends on NUL termination, testing should
> expose it before too long. Wouldn't hurt to mention
David Rowley writes:
> I've attached a patch which builds on the previous patch and relaxes
> the rule that the StringInfo must be NUL-terminated. The rule is
> only relaxed for StringInfos that are initialized with
> initReadOnlyStringInfo.
Yeah, that's probably a reasonable way to frame it.
On Tue, 17 Oct 2023 at 20:39, David Rowley wrote:
>
> On Mon, 16 Oct 2023 at 05:56, Tom Lane wrote:
> > I guess the next question is whether we want to stop here or
> > try to relax the requirement about NUL-termination. I'd be inclined
> > to call that a separate issue deserving a separate comm
On Mon, 16 Oct 2023 at 05:56, Tom Lane wrote:
> * in initStringInfoFromString, str->maxlen must be set to len+1 not len
>
> * comment in exec_bind_message doesn't look like pgindent will like it
>
> * same in record_recv, plus it has a misspelling "Initalize"
>
> * in stringinfo.c, inclusion of pg
David Rowley writes:
> I spent more time on this and did end up with 2 new init functions as
> you mentioned. One for strictly read-only (initReadOnlyStringInfo),
> which cannot be appended to, and as you mentioned, another
> (initStringInfoFromString) which can accept a palloc'd buffer which
> b
On Wed, 11 Oct 2023 at 08:52, Tom Lane wrote:
>
> David Rowley writes:
> > I've attached a slightly more worked on patch that makes maxlen == 0
> > mean read-only. Unsure if a macro is worthwhile there or not.
>
> A few thoughts:
Thank you for the review.
I spent more time on this and did end
David Rowley writes:
> I've attached a slightly more worked on patch that makes maxlen == 0
> mean read-only. Unsure if a macro is worthwhile there or not.
A few thoughts:
* initStringInfoFromStringWithLen() is kind of a mouthful.
How about "initStringInfoWithBuf", or something like that?
* lo
On Mon, 9 Oct 2023 at 16:20, David Rowley wrote:
>
> On Mon, 9 Oct 2023 at 21:17, David Rowley wrote:
> > Here are some more thoughts on how we could improve this:
> >
> > 1. Adjust the definition of StringInfoData.maxlen to define that -1
> > means the StringInfoData's buffer is externally manag
On Tue, 10 Oct 2023 at 06:38, Tom Lane wrote:
> Hm. I'd be inclined to use maxlen == 0 as the indicator of a read-only
> buffer, just because that would not create a problem if we ever want
> to change it to an unsigned type. Other than that, I agree with the
> idea of using a special maxlen val
David Rowley writes:
> On Mon, 9 Oct 2023 at 21:17, David Rowley wrote:
>> Here are some more thoughts on how we could improve this:
>>
>> 1. Adjust the definition of StringInfoData.maxlen to define that -1
>> means the StringInfoData's buffer is externally managed.
>> 2. Adjust enlargeStringInf
On Mon, 9 Oct 2023 at 21:17, David Rowley wrote:
> Here are some more thoughts on how we could improve this:
>
> 1. Adjust the definition of StringInfoData.maxlen to define that -1
> means the StringInfoData's buffer is externally managed.
> 2. Adjust enlargeStringInfo() to add a check for maxlen
On Mon, 9 Oct 2023 at 17:37, Tom Lane wrote:
> Sorry for not having paid more attention to this thread ... but
> I'm pretty desperately unhappy with the patch as-pushed. I agree
> with the criticism that this is a very repetitive coding pattern
> that could have used a macro. But my real problem
David Rowley writes:
> On Thu, 5 Oct 2023 at 21:24, David Rowley wrote:
>> I looked at the patch again and I just couldn't bring myself to change
>> it to that. If it were a macro going into stringinfo.h then I'd agree
>> with having a macro or inline function as it would allow the reader to
>>
On Thu, 5 Oct 2023 at 21:24, David Rowley wrote:
>
> On Thu, 5 Oct 2023 at 18:23, Michael Paquier wrote:
> > Ahem, well. Based on this argument my own argument does not hold
> > much. Perhaps I'd still use a macro at the top of array_userfuncs.c
> > and numeric.c, to avoid repeating the same pa
On Thu, 5 Oct 2023 at 18:23, Michael Paquier wrote:
>
> On Wed, Oct 04, 2023 at 07:47:11PM +1300, David Rowley wrote:
> > The original patch had a new function in stringinfo.c which allowed a
> > StringInfoData to be initialised from an existing string with some
> > given length. Tom wasn't a fan
On Wed, Oct 04, 2023 at 07:47:11PM +1300, David Rowley wrote:
> The original patch had a new function in stringinfo.c which allowed a
> StringInfoData to be initialised from an existing string with some
> given length. Tom wasn't a fan of that because there wasn't any
> protection against someone
Thanks for taking a look at this.
On Wed, 4 Oct 2023 at 16:57, Michael Paquier wrote:
> + buf.len = VARSIZE_ANY_EXHDR(sstate);
> + buf.maxlen = 0;
> + buf.cursor = 0;
>
> Perhaps it would be worth hiding that in a macro defined in
> stringinfo.h?
The original patch had a new fu
On Tue, Oct 03, 2023 at 06:02:10PM +1300, David Rowley wrote:
> I know I said I'd drop this, but I was reminded of it again today. I
> ended up adjusting the patch so that it no longer adds a helper
> function to stringinfo.c and instead just manually assigns the
> StringInfo.data field to point t
On Sun, 12 Feb 2023 at 23:43, David Rowley wrote:
>
> On Sun, 12 Feb 2023 at 19:39, Tom Lane wrote:
> > It could maybe be okay if we added the capability for StringInfoData
> > to understand (and enforce) that its "data" buffer is read-only.
> > However, that'd add overhead to every existing use-
On Sun, 12 Feb 2023 at 19:39, Tom Lane wrote:
> I find this patch horribly dangerous.
I see LogicalRepApplyLoop() does something similar with a
StringInfoData. Maybe it's just scarier having an external function in
stringinfo.c which does this as having it increases the chances of
someone using i
David Rowley writes:
> While working on 16fd03e95, I noticed that in each aggregate
> deserialization function, in order to "receive" the bytea value that
> is the serialized aggregate state, appendBinaryStringInfo is used to
> append the bytes of the bytea value onto a temporary StringInfoData.
>
While working on 16fd03e95, I noticed that in each aggregate
deserialization function, in order to "receive" the bytea value that
is the serialized aggregate state, appendBinaryStringInfo is used to
append the bytes of the bytea value onto a temporary StringInfoData.
Using appendBinaryStringInfo s
28 matches
Mail list logo
