SPI: Correct way to rollback a subtransaction?

Started by Marko Kreenalmost 20 years ago5 messages
#1Marko Kreen
markokr@gmail.com

I want to update one row from possibly several backends. In case
of SERIALIZABLE transactions, the update command may fail. To
hide it from calling transaction, I use a subtransaction and try
to catch and hide the error.

With help of plpgsql source, I wrote following code that _seems_
to work. But I have no idea if it's the correct way to do it:

/* store old state */
MemoryContext oldcontext = CurrentMemoryContext;
ResourceOwner oldowner = CurrentResourceOwner;

BeginInternalSubTransaction(NULL);
res = SPI_connect();
if (res < 0)
elog(ERROR, "cannot connect to SPI");

PG_TRY();
{
res = SPI_execute("update one row", false, 0);
SPI_finish();
ReleaseCurrentSubTransaction();
}
PG_CATCH();
{
SPI_finish();
RollbackAndReleaseCurrentSubTransaction();
FlushErrorState();
res = -1; /* remember failure */
}
PG_END_TRY();

/* restore old state */
MemoryContextSwitchTo(oldcontext);
CurrentResourceOwner = oldowner;

I am suspicious about the ..SubTransaction and SPI_* nesting
and resetting the error state. Can anyone look if it's correct?

Goal of the exercise is to have 8-byte transaction ID's and snapshots
externally available. (Port of xxid module from Slony). Above code
does the update of the 'epoch' table that has only one row.

--
marko

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Marko Kreen (#1)
Re: SPI: Correct way to rollback a subtransaction?

"Marko Kreen" <markokr@gmail.com> writes:

BeginInternalSubTransaction(NULL);
res = SPI_connect();
if (res < 0)
elog(ERROR, "cannot connect to SPI");

PG_TRY();
{
res = SPI_execute("update one row", false, 0);
SPI_finish();
ReleaseCurrentSubTransaction();
}
PG_CATCH();
{
SPI_finish();
RollbackAndReleaseCurrentSubTransaction();
FlushErrorState();
res = -1; /* remember failure */
}
PG_END_TRY();

This seems like a pretty bad idea: if the SPI_connect fails you lose
control without having unwound the subtransaction. That's unlikely,
but still wrong. I think you could do this as

BeginInternalSubTransaction(NULL);
PG_TRY();
{
res = SPI_connect();
if (res < 0)
elog(ERROR, "cannot connect to SPI");
res = SPI_execute("update one row", false, 0);
SPI_finish();
ReleaseCurrentSubTransaction();
}
PG_CATCH();
{
/* we expect rollback to clean up inner SPI call */
RollbackAndReleaseCurrentSubTransaction();
FlushErrorState();
res = -1; /* remember failure */
}
PG_END_TRY();

Check the abort-subtrans path but I think it gets you out of the nested
SPI call. (Because pl_exec.c wants to preserve an already-opened SPI
call, it has to go out of its way to undo this via SPI_restore_connection.
I *think* you don't need that here but am too lazy to check for sure.
Anyway it'll be good practice for you to figure it out for yourself ;-))

regards, tom lane

#3Marko Kreen
markokr@gmail.com
In reply to: Tom Lane (#2)
Re: SPI: Correct way to rollback a subtransaction?

On 2/20/06, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"Marko Kreen" <markokr@gmail.com> writes:

BeginInternalSubTransaction(NULL);
res = SPI_connect();
if (res < 0)
elog(ERROR, "cannot connect to SPI");

This seems like a pretty bad idea: if the SPI_connect fails you lose
control without having unwound the subtransaction. That's unlikely,
but still wrong.

But if I want the error to reach upper transaction? SPI_connect
failure does not seem a 'expected' situation to me.

Or will the started subtransaction corrupt some state?

PG_CATCH();
{
/* we expect rollback to clean up inner SPI call */
RollbackAndReleaseCurrentSubTransaction();
FlushErrorState();
res = -1; /* remember failure */
}
PG_END_TRY();

Check the abort-subtrans path but I think it gets you out of the nested
SPI call. (Because pl_exec.c wants to preserve an already-opened SPI
call, it has to go out of its way to undo this via SPI_restore_connection.
I *think* you don't need that here but am too lazy to check for sure.
Anyway it'll be good practice for you to figure it out for yourself ;-))

Thank you for hints. The RollbackAndReleaseCurrentSubTransaction()
seems to call AbortSubTransaction->AtEOSubXact_SPI() only if the
transaction is TBLOCK_SUBINPROGRESS, As SERIALIZABLE seems to
thow simple elog(ERROR, ...) [executor/execMain.c], and error
handling also does not seem to touch transaction state, it seems
calling SPI_finish() is not needed.

Correct? (Yes, I'm newbie in core code...)

--
marko

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Marko Kreen (#3)
Re: SPI: Correct way to rollback a subtransaction?

"Marko Kreen" <markokr@gmail.com> writes:

On 2/20/06, Tom Lane <tgl@sss.pgh.pa.us> wrote:

This seems like a pretty bad idea: if the SPI_connect fails you lose
control without having unwound the subtransaction. That's unlikely,
but still wrong.

But if I want the error to reach upper transaction? SPI_connect
failure does not seem a 'expected' situation to me.

In that case you should put the SPI_connect and later SPI_finish
*outside* the subtransaction and TRY block. And you'll need
SPI_restore_connection I think. This structure would be exactly
parallel to the way pl_exec.c does it.

regards, tom lane

#5Marko Kreen
markokr@gmail.com
In reply to: Tom Lane (#4)
Re: SPI: Correct way to rollback a subtransaction?

On 2/21/06, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"Marko Kreen" <markokr@gmail.com> writes:

On 2/20/06, Tom Lane <tgl@sss.pgh.pa.us> wrote:

This seems like a pretty bad idea: if the SPI_connect fails you lose
control without having unwound the subtransaction. That's unlikely,
but still wrong.

But if I want the error to reach upper transaction? SPI_connect
failure does not seem a 'expected' situation to me.

In that case you should put the SPI_connect and later SPI_finish
*outside* the subtransaction and TRY block. And you'll need
SPI_restore_connection I think. This structure would be exactly
parallel to the way pl_exec.c does it.

It does not seem worth the complexity, I rather go with the simple
approach and put it inside TRY block then.

--
marko