Re: SPI: Correct way to rollback a subtransaction?

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "Marko Kreen" <markokr(at)gmail(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: SPI: Correct way to rollback a subtransaction?
Date: 2006-02-20 15:35:41
Message-ID: 26311.1140449741@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

"Marko Kreen" <markokr(at)gmail(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tatsuo Ishii 2006-02-20 15:46:13 Re: possible design bug with PQescapeString()
Previous Message Philip Warner 2006-02-20 15:34:40 Re: new feature: LDAP database name resolution