Michael Paquier <mich...@paquier.xyz> writes:
> On Fri, Jul 30, 2021 at 08:57:35AM +0000, liuhuail...@fujitsu.com wrote:
>> When I used SPI_execute_plan function on PG12 which commit 41c6a5be is used,
>> Segment fault occurred.

> I see nothing wrong in this commit, FWIW.
> But I see an issue with your code.  It seems to me that you should
> push a snapshot before doing SPI_prepare() and SPI_execute_plan(),
> as of:
> PushActiveSnapshot(GetTransactionSnapshot()); 

Yes.  What that commit did was to formalize the requirement that
a snapshot exist *before* entering SPI.  Before that, you might
have gotten away without one, depending on what you were trying
to do (in particular, detoasting a toasted output Datum would
fail if you lack an external snapshot).

This isn't the first complaint we've seen from somebody whose
code used to work and now fails there, however.  I wonder if
we should convert the Assert into an actual test-and-elog, say

        /* Otherwise, we'd better have an active Portal */
        portal = ActivePortal;
-       Assert(portal != NULL);
+       if (unlikely(portal == NULL))
+               elog(ERROR, "must have an outer snapshot or portal");
        Assert(portal->portalSnapshot == NULL);

Perhaps that would help people to realize that the bug is theirs
not EnsurePortalSnapshotExists's.

I gather BTW that the OP is trying to test C code in a non-assert
build.  Not a great approach.

                        regards, tom lane


Reply via email to