Thread

Commits

  1. Remove plpgsql's special-case code paths for SET/RESET.

  2. Restore the portal-level snapshot after procedure COMMIT/ROLLBACK.

  3. Avoid detoasting failure after COMMIT inside a plpgsql FOR loop.

  1. BUG #15990: PROCEDURE throws "SQL Error [XX000]: ERROR: no known snapshots" with PostGIS geometries

    The Post Office <noreply@postgresql.org> — 2019-09-04T10:06:16Z

    The following bug has been logged on the website:
    
    Bug reference:      15990
    Logged by:          Andreas Wicht
    Email address:      a.wicht@gmail.com
    PostgreSQL version: 11.5
    Operating system:   Ubuntu 18.04
    Description:        
    
    Hi there,
    
    I am not sure where to place this problem, here or at the PostGIS mailing
    list. I'd like to start here though.
    I have a function which needs a commit after each loop (inserting a result
    into a target table). So far I worked around this requirement with dblink.
    When the new procedures were implemented I tried to port the function to a
    procedure, greatly reducing the complexity.
    While testing I started to get the above mentioned error. 
    I could dumb the procedure down to the very basics to reproduce the error.
    
    Note that the procedure fails as soon as the geometry column is part of the
    SELECT statement defining the FOR loop.
    Researching this error did not yield any useful information to me (at least
    none which is evident to me).
    
    Steps to reproduce:
    CREATE EXTENSION postgis;
    CREATE SCHEMA temp;
    
    wget
    https://www.statistik-berlin-brandenburg.de/opendata/RBS_OD_ORT_2016_12.zip
    unzip RBS_OD_ORT_2016_12.zip
    shp2pgsql -I -g geom -s 25833 RBS_OD_ORT_2016_12.shp temp.test | psql -h XXX
    -p XXX -d XXX -U XXX
    
    CREATE TABLE temp.mytable (gid integer, geom geometry);
    
    CREATE OR REPLACE PROCEDURE temp.testprocedure(polygon_tbl regclass)
    AS $$
    DECLARE
        _poly_tbl   ALIAS FOR $1;
        _rcd        RECORD;
    BEGIN
        FOR _rcd IN
            EXECUTE format ('SELECT gid, geom FROM %s', _poly_tbl)
        LOOP
            INSERT INTO temp.mytable (gid, geom) VALUES (_rcd.gid, _rcd.geom);
            COMMIT;
        END LOOP;
    END;
    $$
    LANGUAGE plpgsql;
    
    CALL temp.testprocedure('temp.test');
    
    ---------
    PostGIS version:
    POSTGIS="2.5.2 r17328" [EXTENSION] PGSQL="110" GEOS="3.6.2-CAPI-1.10.2
    4d2925d6" PROJ="Rel. 4.9.3, 15 August 2016" GDAL="GDAL 2.2.3, released
    2017/11/20" LIBXML="2.9.4" LIBJSON="0.12.1" LIBPROTOBUF="1.2.1" TOPOLOGY
    RASTER
    
    PostgeSQL version:
    PostgreSQL 11.5 (Ubuntu 11.5-1.pgdg18.04+1) on x86_64-pc-linux-gnu, compiled
    by gcc (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0, 64-bit
    
    Greetings
    Andreas
    
    
  2. Re: BUG #15990: PROCEDURE throws "SQL Error [XX000]: ERROR: no known snapshots" with PostGIS geometries

    Andres Freund <andres@anarazel.de> — 2019-09-04T10:56:18Z

    Hi,
    
    On 2019-09-04 10:06:16 +0000, PG Bug reporting form wrote:
    > Note that the procedure fails as soon as the geometry column is part of the
    > SELECT statement defining the FOR loop.
    > Researching this error did not yield any useful information to me (at least
    > none which is evident to me).
    
    The error is from:
    static void
    init_toast_snapshot(Snapshot toast_snapshot)
    {
    	Snapshot	snapshot = GetOldestSnapshot();
    
    	if (snapshot == NULL)
    		elog(ERROR, "no known snapshots");
    
    	InitToastSnapshot(*toast_snapshot, snapshot->lsn, snapshot->whenTaken);
    }
    
    
    > CREATE OR REPLACE PROCEDURE temp.testprocedure(polygon_tbl regclass)
    > AS $$
    > DECLARE
    >     _poly_tbl   ALIAS FOR $1;
    >     _rcd        RECORD;
    > BEGIN
    >     FOR _rcd IN
    >         EXECUTE format ('SELECT gid, geom FROM %s', _poly_tbl)
    >     LOOP
    >         INSERT INTO temp.mytable (gid, geom) VALUES (_rcd.gid, _rcd.geom);
    >         COMMIT;
    >     END LOOP;
    > END;
    > $$
    > LANGUAGE plpgsql;
    
    Hm. I don't immediately see anything here that could really be postgis
    specific. I assume it's just because the geom datum is large and gets
    toasted.  A bit of playing shows that it can be reproduced without:
    
    CREATE TABLE toasted(id serial primary key, data text);
    INSERT INTO toasted(data) VALUES((SELECT string_agg(random()::text, ':') FROM generate_series(1, 1000)));
    INSERT INTO toasted(data) VALUES((SELECT string_agg(random()::text, ':') FROM generate_series(1, 1000)));
    INSERT 0 1
    
    DO $$ DECLARE v_r record; BEGIN FOR v_r in SELECT data FROM toasted LOOP INSERT INTO toasted(data) VALUES(v_r.data);COMMIT;END LOOP;END;$$;
    
    ERROR:  XX000: no known snapshots
    CONTEXT:  PL/pgSQL function inline_code_block line 1 at FOR over SELECT rows
    LOCATION:  init_toast_snapshot, tuptoaster.c:2416
    
    Note that there's no errors if there's only one already in the table,
    not if all the data is inserted without being sourced from a table.
    
    This looks like it might be a procedure related bug to me. Peter?
    
    The backtrace in my lightly modified tree is:
    
    #0  init_toast_snapshot (toast_snapshot=0x7ffd5dc53280)
        at /home/andres/src/postgresql/src/backend/access/heap/tuptoaster.c:2416
    #1  0x000055ee5a7fc0ef in toast_fetch_datum (attr=0x55ee5d155a78)
        at /home/andres/src/postgresql/src/backend/access/heap/tuptoaster.c:1930
    #2  0x000055ee5a7f8bb2 in heap_tuple_fetch_attr (attr=0x55ee5d155a78)
        at /home/andres/src/postgresql/src/backend/access/heap/tuptoaster.c:108
    #3  0x000055ee5a7fad29 in toast_flatten_tuple (tup=0x55ee5d155a48,
        tupleDesc=0x55ee5d14f510)
        at /home/andres/src/postgresql/src/backend/access/heap/tuptoaster.c:1110
    #4  0x000055ee5ac77d32 in expanded_record_set_tuple (erh=0x55ee5d14f3f8,
        tuple=0x55ee5d155a48, copy=true, expand_external=true)
        at /home/andres/src/postgresql/src/backend/utils/adt/expandedrecord.c:473
    #5  0x00007f4450307cef in exec_for_query (estate=0x7ffd5dc57920, stmt=0x55ee5d154b00,
        portal=0x55ee5d09e040, prefetch_ok=true)
        at /home/andres/src/postgresql/src/pl/plpgsql/src/pl_exec.c:5970
    #6  0x00007f4450301984 in exec_stmt_fors (estate=0x7ffd5dc57920, stmt=0x55ee5d154b00)
        at /home/andres/src/postgresql/src/pl/plpgsql/src/pl_exec.c:2791
    #7  0x00007f44502ffedc in exec_stmt (estate=0x7ffd5dc57920, stmt=0x55ee5d154b00)
        at /home/andres/src/postgresql/src/pl/plpgsql/src/pl_exec.c:1997
    #8  0x00007f44502ffc94 in exec_stmts (estate=0x7ffd5dc57920, stmts=0x55ee5d154ef0)
        at /home/andres/src/postgresql/src/pl/plpgsql/src/pl_exec.c:1924
    #9  0x00007f44502ffb40 in exec_stmt_block (estate=0x7ffd5dc57920, block=0x55ee5d154f28)
        at /home/andres/src/postgresql/src/pl/plpgsql/src/pl_exec.c:1865
    #10 0x00007f44502ffdce in exec_stmt (estate=0x7ffd5dc57920, stmt=0x55ee5d154f28)
        at /home/andres/src/postgresql/src/pl/plpgsql/src/pl_exec.c:1957
    #11 0x00007f44502fd542 in plpgsql_exec_function (func=0x55ee5d149a98,
        fcinfo=0x7ffd5dc57b60, simple_eval_estate=0x55ee5d125448, atomic=false)
        at /home/andres/src/postgresql/src/pl/plpgsql/src/pl_exec.c:589
    #12 0x00007f44502f7d58 in plpgsql_inline_handler (fcinfo=0x7ffd5dc57c40)
        at /home/andres/src/postgresql/src/pl/plpgsql/src/pl_handler.c:339
    #13 0x000055ee5adab248 in FunctionCall1Coll (flinfo=0x7ffd5dc57ca0, collation=0, arg1=94482251264896) at /home/andres/src/postgresql/src/backend/utils/fmgr/fmgr.c:1140
    #14 0x000055ee5adabde6 in OidFunctionCall1Coll (functionId=13404, collation=0, arg1=94482251264896) at /home/andres/src/postgresql/src/backend/utils/fmgr/fmgr.c:1418
    #15 0x000055ee5a981ab1 in ExecuteDoStmt (stmt=0x55ee5d037070, atomic=false) at /home/andres/src/postgresql/src/backend/commands/functioncmds.c:2266
    #16 0x000055ee5ac265e8 in standard_ProcessUtility (pstmt=0x55ee5d037370,
        queryString=0x55ee5d0363a8 "DO $$ DECLARE v_r record; BEGIN FOR v_r in SELECT data FROM toasted LOOP INSERT INTO toasted(data) VALUES(v_r.data);COMMIT;END LOOP;END;$$;",
        context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, dest=0x55ee5d037440, completionTag=0x7ffd5dc58140 "") at /home/andres/src/postgresql/src/backend/tcop/utility.c:523
    #17 0x000055ee5ac26123 in ProcessUtility (pstmt=0x55ee5d037370,
        queryString=0x55ee5d0363a8 "DO $$ DECLARE v_r record; BEGIN FOR v_r in SELECT data FROM toasted LOOP INSERT INTO toasted(data) VALUES(v_r.data);COMMIT;END LOOP;END;$$;",
        context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, dest=0x55ee5d037440, completionTag=0x7ffd5dc58140 "") at /home/andres/src/postgresql/src/backend/tcop/utility.c:360
    #18 0x000055ee5ac24f8a in PortalRunUtility (portal=0x55ee5d09df28, pstmt=0x55ee5d037370, isTopLevel=true, setHoldSnapshot=false, dest=0x55ee5d037440,
        completionTag=0x7ffd5dc58140 "") at /home/andres/src/postgresql/src/backend/tcop/pquery.c:1175
    #19 0x000055ee5ac251ae in PortalRunMulti (portal=0x55ee5d09df28, isTopLevel=true, setHoldSnapshot=false, dest=0x55ee5d037440, altdest=0x55ee5d037440,
        completionTag=0x7ffd5dc58140 "") at /home/andres/src/postgresql/src/backend/tcop/pquery.c:1321
    #20 0x000055ee5ac246ba in PortalRun (portal=0x55ee5d09df28, count=9223372036854775807, isTopLevel=true, run_once=true, dest=0x55ee5d037440, altdest=0x55ee5d037440,
        completionTag=0x7ffd5dc58140 "") at /home/andres/src/postgresql/src/backend/tcop/pquery.c:796
    #21 0x000055ee5ac1e0b8 in exec_simple_query (
        query_string=0x55ee5d0363a8 "DO $$ DECLARE v_r record; BEGIN FOR v_r in SELECT data FROM toasted LOOP INSERT INTO toasted(data) VALUES(v_r.data);COMMIT;END LOOP;END;$$;")
        at /home/andres/src/postgresql/src/backend/tcop/postgres.c:1231
    #22 0x000055ee5ac2276a in PostgresMain (argc=1, argv=0x55ee5d05c758, dbname=0x55ee5d05c6a0 "postgres", username=0x55ee5d032918 "andres")
        at /home/andres/src/postgresql/src/backend/tcop/postgres.c:4256
    #23 0x000055ee5ab72e74 in BackendRun (port=0x55ee5d057b00) at /home/andres/src/postgresql/src/backend/postmaster/postmaster.c:4446
    #24 0x000055ee5ab725ce in BackendStartup (port=0x55ee5d057b00) at /home/andres/src/postgresql/src/backend/postmaster/postmaster.c:4137
    #25 0x000055ee5ab6e702 in ServerLoop () at /home/andres/src/postgresql/src/backend/postmaster/postmaster.c:1704
    #26 0x000055ee5ab6df34 in PostmasterMain (argc=37, argv=0x55ee5d030290) at /home/andres/src/postgresql/src/backend/postmaster/postmaster.c:1377
    #27 0x000055ee5aa7bb76 in main (argc=37, argv=0x55ee5d030290) at /home/andres/src/postgresql/src/backend/main/main.c:210
    
    
    Which seems to suggest that the snapshot management for procedures
    (possibly not even just plpgsql), isn't quite right.
    
    Greetings,
    
    Andres Freund
    
    
    
    
  3. Re: BUG #15990: PROCEDURE throws "SQL Error [XX000]: ERROR: no known snapshots" with PostGIS geometries

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-05-11T23:01:34Z

    Andres Freund <andres@anarazel.de> writes:
    > This looks like it might be a procedure related bug to me. Peter?
    
    Somebody reported this same bug again today [1] ... why has it still
    not been dealt with?
    
    After an admittedly cursory look-around, it seems like the problem
    can be stated as "init_toast_snapshot expects that there already
    is a transaction snapshot, which there is not because we just
    committed and nothing has re-established a transaction snapshot".
    So the question is, where shall we force a new transaction snapshot
    to be created after a COMMIT/ROLLBACK inside a procedure?
    
    The most localized fix would be to let init_toast_snapshot itself
    do that, but that seems like a bit of a layering violation; plus
    I'm not quite convinced that's the only place with the issue.
    (However, it *is* the only caller of GetOldestSnapshot() AFAICS.
    So maybe everyplace else is calling GetTransactionSnapshot() to
    begin with, in which case this could do likewise I should think.)
    
    It really seems to me like _SPI_commit() should have started a
    new transaction --- why is it okay to delay that?
    
    BTW, why is this not caught by the plpgsql-toast.spec isolation test?
    Seems like that's doing almost exactly the same thing as the trouble
    reports.
    
    			regards, tom lane
    
    [1] https://www.postgresql.org/message-id/dae29212-ad31-8701-ef16-dd7420bfaa56%40perfexpert.ch
    
    
    
    
  4. Re: BUG #15990: PROCEDURE throws "SQL Error [XX000]: ERROR: no known snapshots" with PostGIS geometries

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-05-12T02:01:11Z

    [ Roping Robert into this, as committer of 3e2f3c2e4 ]
    
    I wrote:
    > After an admittedly cursory look-around, it seems like the problem
    > can be stated as "init_toast_snapshot expects that there already
    > is a transaction snapshot, which there is not because we just
    > committed and nothing has re-established a transaction snapshot".
    > So the question is, where shall we force a new transaction snapshot
    > to be created after a COMMIT/ROLLBACK inside a procedure?
    
    > The most localized fix would be to let init_toast_snapshot itself
    > do that, but that seems like a bit of a layering violation; plus
    > I'm not quite convinced that's the only place with the issue.
    
    I tried this, which leads to a nicely small patch and seems to resolve
    the existing reports, but now I'm not sure that it's actually safe.
    I think the bigger-picture question is, if we're trying to detoast
    as the first step in a new transaction of a procedure, where's the
    guarantee that the TOAST data still exists to be fetched?  For sure
    we aren't holding any locks that would stop VACUUM from reclaiming
    recently-dead TOAST rows.
    
    In a recent discussion at [1], Konstantin Knizhnik reasoned that the
    problem is that plpgsql is holding rows that it's prefetched but not
    yet detoasted, and proposed disabling prefetch to solve this.  I think
    he's probably right, although his patch strikes me as both overcomplicated
    and wrong.  I suspect we must disable prefetch in any non-atomic
    execution context, because we can't know whether a COMMIT will be executed
    by some called procedure.
    
    I'm still wondering why plpgsql-toast.spec is failing to show the
    problem, too.
    
    			regards, tom lane
    
    [1] https://www.postgresql.org/message-id/flat/03644c0e6bb82132ac783982b6abffdf%40postgrespro.ru
    
    
  5. Re: BUG #15990: PROCEDURE throws "SQL Error [XX000]: ERROR: no known snapshots" with PostGIS geometries

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-05-12T03:25:57Z

    I wrote:
    > I think the bigger-picture question is, if we're trying to detoast
    > as the first step in a new transaction of a procedure, where's the
    > guarantee that the TOAST data still exists to be fetched?  For sure
    > we aren't holding any locks that would stop VACUUM from reclaiming
    > recently-dead TOAST rows.
    
    Yeah.  So here's a patch I actually believe in to some extent, based
    on Konstantin's idea.
    
    > I'm still wondering why plpgsql-toast.spec is failing to show the
    > problem, too.
    
    The answer to that is that it's not testing commit-inside-a-cursor-loop,
    and if it were, it'd still fail to show the problem because its test
    table contains just one row.
    
    Interestingly, if you try the test case added here without adding the
    code patch, you get a "missing chunk number ... for toast value ..."
    error, not "no known snapshots".  I think that's because the test
    case has additional commands after the COMMIT, causing the transaction
    snapshot to get re-established.
    
    			regards, tom lane
    
    
  6. Re: BUG #15990: PROCEDURE throws "SQL Error [XX000]: ERROR: no known snapshots" with PostGIS geometries

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-05-12T15:37:46Z

    ... okay, now I'm roping Alvaro into this thread, because the attached
    test case (extracted from [1]) shows that there's still a problem,
    and this time it seems like we are dropping the ball on snapshot
    management.
    
    The sequence of events here is that after the first COMMIT inside the
    loop, we call _SPI_execute_plan to execute the "select txt into t from
    test2 where i=r.i;".  It does what it's supposed to, i.e.
    
                PushActiveSnapshot(GetTransactionSnapshot());
                ... run query ...
                PopActiveSnapshot();
    
    and then hands back a tuple that includes a toasted datum.  plpgsql
    knows it must detoast that value before storing it into "t", but
    when it calls the toaster, GetOldestSnapshot returns NULL because
    we have neither any "active" nor any "registered" snapshots.
    
    ISTM there are two ways we could look at this:
    
    1. COMMIT is dropping the ball by not forcing there to be any
    registered transaction-level snapshot afterward.  (Maybe it's
    not exactly COMMIT that must do this, but in any case the
    snapshot situation after COMMIT is clearly different from
    normal running, and that seems highly bug-prone.)
    
    2. GetOldestSnapshot ought to be willing to fall back to
    CurrentSnapshot if FirstSnapshotSet is true but there are
    no active or registered snapshots.  But it's not clear how
    its promises about returning the "oldest" snapshot would apply.
    
    Thoughts?
    
    			regards, tom lane
    
    [1] https://www.postgresql.org/message-id/65424747-42ed-43d5-4cca-6b03481409a4%40perfexpert.ch
    
    
  7. Re: BUG #15990: PROCEDURE throws "SQL Error [XX000]: ERROR: no known snapshots" with PostGIS geometries

    Andres Freund <andres@anarazel.de> — 2021-05-12T15:51:53Z

    Hi,
    
    On 2021-05-12 11:37:46 -0400, Tom Lane wrote:
    > ISTM there are two ways we could look at this:
    > 
    > 1. COMMIT is dropping the ball by not forcing there to be any
    > registered transaction-level snapshot afterward.  (Maybe it's
    > not exactly COMMIT that must do this, but in any case the
    > snapshot situation after COMMIT is clearly different from
    > normal running, and that seems highly bug-prone.)
    > 
    > 2. GetOldestSnapshot ought to be willing to fall back to
    > CurrentSnapshot if FirstSnapshotSet is true but there are
    > no active or registered snapshots.  But it's not clear how
    > its promises about returning the "oldest" snapshot would apply.
    
    FirstSnapshotSet doesn't indicate the snapshot is usable, unless
    IsolationUsesXactSnapshot() - in which case it also registers the
    snapshot. We don't maintain MyProc->xmin outside of that, which I think
    means we can't rely on the snapshot at all?  Or am I missing something?
    
    Greetings,
    
    Andres Freund
    
    
    
    
  8. Re: BUG #15990: PROCEDURE throws "SQL Error [XX000]: ERROR: no known snapshots" with PostGIS geometries

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-05-12T16:00:10Z

    Andres Freund <andres@anarazel.de> writes:
    > On 2021-05-12 11:37:46 -0400, Tom Lane wrote:
    >> ISTM there are two ways we could look at this:
    >> 
    >> 1. COMMIT is dropping the ball by not forcing there to be any
    >> registered transaction-level snapshot afterward.  (Maybe it's
    >> not exactly COMMIT that must do this, but in any case the
    >> snapshot situation after COMMIT is clearly different from
    >> normal running, and that seems highly bug-prone.)
    >> 
    >> 2. GetOldestSnapshot ought to be willing to fall back to
    >> CurrentSnapshot if FirstSnapshotSet is true but there are
    >> no active or registered snapshots.  But it's not clear how
    >> its promises about returning the "oldest" snapshot would apply.
    
    > FirstSnapshotSet doesn't indicate the snapshot is usable, unless
    > IsolationUsesXactSnapshot() - in which case it also registers the
    > snapshot. We don't maintain MyProc->xmin outside of that, which I think
    > means we can't rely on the snapshot at all?  Or am I missing something?
    
    Yeah, on further thought, the real question to be asking here is
    "what's protecting that in-flight datum from becoming a dangling
    pointer?".  AFAICT, the answer right now is "nothing".  Therefore,
    it is *never* okay for plpgsql to be executing without a registered
    transaction snapshot; and it seems quite unlikely that it'd be any
    safer for any other PL.
    
    I'm batting the ball back into Peter's court.
    
    			regards, tom lane
    
    
    
    
  9. Re: BUG #15990: PROCEDURE throws "SQL Error [XX000]: ERROR: no known snapshots" with PostGIS geometries

    Andres Freund <andres@anarazel.de> — 2021-05-12T16:13:54Z

    Hi,
    
    On 2021-05-12 12:00:10 -0400, Tom Lane wrote:
    > Yeah, on further thought, the real question to be asking here is
    > "what's protecting that in-flight datum from becoming a dangling
    > pointer?".  AFAICT, the answer right now is "nothing".  Therefore,
    > it is *never* okay for plpgsql to be executing without a registered
    > transaction snapshot; and it seems quite unlikely that it'd be any
    > safer for any other PL.
    
    A bit independently of this concrete issue, I wonder if we could make it
    easier to detect such bugs. If we somehow™ associated an xmin horizon
    with toasted datums, we could then assert before dereferencing that we
    still guarantee that such a reference is still valid. The "somehow" part
    obviously isn't entirely trivial, but it seems we could hack it by
    encoding it as a different vartag?
    
    Greetings,
    
    Andres Freund
    
    
    
    
  10. Re: BUG #15990: PROCEDURE throws "SQL Error [XX000]: ERROR: no known snapshots" with PostGIS geometries

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-05-18T19:45:04Z

    I wrote:
    > Yeah, on further thought, the real question to be asking here is
    > "what's protecting that in-flight datum from becoming a dangling
    > pointer?".  AFAICT, the answer right now is "nothing".  Therefore,
    > it is *never* okay for plpgsql to be executing without a registered
    > transaction snapshot; and it seems quite unlikely that it'd be any
    > safer for any other PL.
    
    Here's a draft patch for this issue.
    
    I concluded that the reason things work for a normal DO block
    or plpgsql procedure (without internal transaction management)
    is that pquery.c -- in particular, PortalRunUtility -- establishes
    a snapshot for the duration of execution of the portal that is
    running the CALL or DO.  That allows us to detoast the results of
    a SPI query even though SPI has released whatever statement-level
    snapshot it was using.  Hence, what we should do to fix this is
    to try to restore that state of affairs after an internal commit.
    We can't just make a snapshot instantly, because it may be that
    the user wants to issue SET command(s) to change the transaction
    mode and/or LOCK commands to acquire locks before a new
    transaction snapshot is taken.  This means that _SPI_execute_plan
    needs to deal with re-creating the Portal-level snapshot, but not
    till just before executing a command that has to have a snapshot.
    
    (Of course, fixing it in _SPI_execute_plan only fixes things for
    PLs that go through SPI.  But if you don't, you were pretty much
    on your own before, and you still are.)
    
    So the attached patch removes PortalRunUtility's local state in
    favor of adding a Portal field that tracks the snapshot provided
    by the Portal, if any.  Then when we make a new snapshot that
    logically substitutes for the portal's original snapshot, we can
    store it in that field and it'll be released when we finally
    exit the portal.
    
    There's a loose end that this doesn't really resolve, which is
    that a procedure could return output data that involves a toast
    pointer.  If the procedure's last action was COMMIT, we'll
    come back to ExecuteCallStmt with no snapshot that protects the
    validity of that toast pointer.  For the moment I just added a
    comment that says PLs mustn't do that.  I think that is safe
    for plpgsql, because it treats output parameters like local
    variables, and it already knows it mustn't store toast pointers
    in procedure local variables.  It's probably all right for our
    other PLs too, because they don't really deal in direct use of
    toasted datums anyway.  But it still seems scary, because there is
    really no good way to detect a violation.
    
    A couple of other bits of unfinished business:
    
    * This basically moves all of the snapshot management logic into
    _SPI_execute_plan, making "no_snapshots" quite a misnomer.
    It kind of was already, because of its contribution to whether
    we pass down PROCESS_UTILITY_QUERY_NONATOMIC, but now it's a
    flat out lie.  We still need a flag to control what happens with
    PROCESS_UTILITY_QUERY_NONATOMIC, so I renamed it to
    allow_nonatomic.  For the moment I just did that renaming locally
    in _SPI_execute_plan.  In HEAD, we ought to propagate that name
    into SPIExecuteOptions, but I haven't done that here.
    
    * I think we could remove the PLPGSQL_STMT_SET infrastructure
    and go back to treating SET/RESET the same as other utility
    commands so far as plpgsql is concerned.  I haven't done that
    here either, because it wouldn't be appropriate to do in the
    back branches (if only for fear of breaking pldebugger).
    
    Note that we still need the other patch to discourage plpgsql
    from leaving not-yet-detoasted datums hanging around in a
    FOR loop's fetch queue.
    
    			regards, tom lane