Re: [HACKERS] Transaction control in procedures

On 12/7/17 18:47, Andrew Dunstan wrote:
> Referring to anonymous blocks might be a bit mystifying for some
> readers, unless we note that they are invoked via DO.

Added parenthetical comments.

> I think this sentence should probably be worded a bit differently:
>
> (And of course, BEGIN and END have different meanings in PL/pgSQL.)
>
> Perhaps "Note that" instead of "And of course,".

fixed

> Why aren't the SPI functions that error out or return 0 just void
> instead of returning an int?

Tried to align then with existing functions, but I agree it seems weird.
Changed to return void.

> The docs say SPI_set_non_atomic() returns 0 on success, but the code
> says otherwise.

Fixed the documentation.

> This sentence in the comment in SPI_Commit() is slightly odd:
>
> This restriction is particular to PLs implemented on top of SPI.
>
> Perhaps "required by" rather than "particular to" would make it read better.

fixed

> The empty free_commit() and free_rollback() functions in pl_funcs.c look
> slightly odd. I realize that the compiler should optimize the calls
> away, but it seems an odd style.

That's how the existing code for other statements looks as well.

> One other thing I wondered about was what if a PL function (say plperl)
> used SPI to open an explicit cursor and then looped over it? If there
> were a commit or rollback inside that loop we wouldn't have the same
> protections we have in plpgsql, ISTM. I haven't tried this yet, so I'm
> just speculating about what might happen.

Good point. I added test cases similar to the plpgsql tests to the
other three languages, which not-so-amusingly gave three different
outcomes. In PL/Perl in particular, the commit clears away the portal,
and the next call to spi_fetchrow() will then not find the cursor and
just return undefined. So that is not so nice. I'm thinking about
extending the portal pinning mechanism to the other languages as well,
which seems mildly useful independent of transaction management. I will
propose a patch for that soon.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services