> On Aug 23, 2018, at 9:34 AM, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:
>
> I think I've found a reasonable fix for this.
>
> The problem arises with the combination of CALL with output parameters
> and doing a COMMIT inside the procedure. When a CALL has output
> parameters, the portal uses the strategy PORTAL_UTIL_SELECT instead of
> PORTAL_MULTI_QUERY. Using PORTAL_UTIL_SELECT causes the portal's
> snapshot to be registered with the current resource
> owner (portal->holdSnapshot); see
> 9ee1cf04ab6bcefe03a11837b53f29ca9dc24c7a for the reason.
>
> Normally, PortalDrop() unregisters the snapshot. If not, then
> ResourceOwnerRelease() will print a warning about a snapshot leak on
> transaction commit. A transaction commit normally drops all
> portals (PreCommit_Portals()), except the active portal. So in case of
> the active portal, we need to manually release the snapshot to avoid the
> warning.
>
> PreCommit_Portals() already contains some code that deals specially with
> the active portal versus resource owners, so it seems reasonable to add
> there.
>
> I think this problem could theoretically apply to any
> multiple-transaction utility command. For example, if we had a variant
> of VACUUM that returned tuples, it would produce the same warning.
> (This patch would fix that as well.)
Applied the patch against head. make check passed, all the tests I ran
earlier in this thread passed as well.
Reviewed the code - looks to match above reasoning, and comments
are clear.
From my perspective it looks good, but happy to hear from someone
more familiar than me with this area of the code.
Thanks,
Jonathan