Re: Decoding speculative insert with toast leaks memory - Mailing list pgsql-hackers

From Tomas Vondra
Subject Re: Decoding speculative insert with toast leaks memory
Date
Msg-id 7dab0929-a966-0c0a-4726-878fced2fe00@enterprisedb.com
Whole thread Raw
In response to Re: Decoding speculative insert with toast leaks memory  (Amit Kapila <amit.kapila16@gmail.com>)
Responses Re: Decoding speculative insert with toast leaks memory
List pgsql-hackers
Hi,

On 6/18/21 5:50 AM, Amit Kapila wrote:
> On Thu, Jun 17, 2021 at 2:55 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>>
>> Your patch looks good to me as well. I would like to retain the
>> comment as it is from master for now. I'll do some testing and push it
>> tomorrow unless there are additional comments.
>>
> 
> Pushed!
> 

While rebasing a patch broken by 4daa140a2f5, I noticed that the patch
does this:

@@ -63,6 +63,7 @@ enum ReorderBufferChangeType
        REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID,
        REORDER_BUFFER_CHANGE_INTERNAL_SPEC_INSERT,
        REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM,
+       REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT,
        REORDER_BUFFER_CHANGE_TRUNCATE
 };


I understand adding the ABORT right after CONFIRM

Isn't that an undesirable ABI break for extensions? It changes the value
for the TRUNCATE item, so if an extension references to that somehow
it'd suddenly start failing (until it gets rebuilt). And the failures
would be pretty confusing and seemingly contradicting the code.

FWIW I don't know how likely it is for an extension to depend on the
TRUNCATE value (it'd be far worse for INSERT/UPDATE/DELETE), but seems
moving the new element at the end would solve this.


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



pgsql-hackers by date:

Previous
From: Daniel Gustafsson
Date:
Subject: Re: SSL Tests for sslinfo extension
Next
From: Peter Geoghegan
Date:
Subject: Re: PG 14 release notes, first draft