Re: minor improvement in snapbuild: use existing interface and remove fake code

ocean_li_996 <ocean_li_996@163.com>

From: ocean_li_996 <ocean_li_996@163.com>
To: "Yuefei Shi" <shiyuefei1004@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org, "mohen.lhy@alibaba-inc.com" <mohen.lhy@alibaba-inc.com>
Date: 2025-11-18T02:05:18Z
Lists: pgsql-hackers
Hi yuefei,


Thanks for your review.


At 2025-11-18 09:13:12, "Yuefei Shi" <shiyuefei1004@gmail.com> 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