Re: Decoding speculative insert with toast leaks memory

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