Hi, On 6/18/21 5:50 AM, Amit Kapila wrote: > On Thu, Jun 17, 2021 at 2:55 PM Amit Kapila <amit.kapil...@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