Re: BUG #19839: Assert in ReorderBufferRestoreCleanup for aborted, spilled transaction

Hi,

On Mon, Nov 09, 2026 at 07:46:59PM +0800, Zhijie Hou wrote:
> Thanks for the review. It looks like Atisss's update patches didn't reach
> the list, probably a missing Reply-All, so I'm helping to attach the same
> patches (provided by Atisss) here.

LGTM, just one nit about:

+ /*
+ * Mark the transaction as serialized only if something was written,
+ * and only after the check above. final_lsn is advanced for each
+ * change written, so a transaction that wrote nothing, such as a
+ * subtransaction without decoded changes, would be marked with an
+ * invalid final_lsn, and ReorderBufferRestoreCleanup() would later
+ * fail on it.
+ */

It looks lengthy and “the check above” is ambiguous. I'm not sure we need a new
comment here, maybe just replace the existing?

/* update the statistics iff we have spilled anything */

with

/* update the statistics and mark the transaction serialized iff we spilled */

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com