Hi yuefei,
Thanks for your review. At 2025-11-18 09:13:12, "Yuefei Shi" <[email protected]> wrote: > - /* only build a new snapshot if we don't have a prebuilt one */ > - if (builder->snapshot == NULL) > - { > - builder->snapshot = SnapBuildBuildSnapshot(builder); > - /* increase refcount for the snapshot builder */ > - SnapBuildSnapIncRefcount(builder->snapshot); > - } > + Snapshot snapshot = SnapBuildGetOrBuildSnapshot(builder); > > /* > * Increase refcount for the transaction we're handing the snapshot > * out to. > */ > - SnapBuildSnapIncRefcount(builder->snapshot); > + SnapBuildSnapIncRefcount(snapshot); > ReorderBufferSetBaseSnapshot(builder->reorder, xid, lsn, > - builder->snapshot); > + snapshot); > > The snapshot created above is a temporary variable and is not recorded into > builder->snapshot, which may cause a leak. AFAICT, the snapshot is created and recorded into builder->snapshot in SnapBuildGetOrBuildSnapshot() if needed. And the local variable snapshot is just a poniter which actually pointing to builder->snapshot. Did I missed something? Regards, Haiyang Li
