Re: Open 7.4 items

Stephan Szabo <sszabo(at)megazone(dot)bigpanda(dot)com> writes:
> It's not cleaned up, but yes. It appears to work for the simple tests I've
> done and should fall back if the permissions don't work to do a single
> query on both tables.

Here's my code-reviewed version of the patch. Anyone else want to take
a look?

> I wasn't sure what to do about some of the spi error conditions. For many
> of them I'm just returning false now so that it will try the other
> mechanism in case that might work. I'm not really sure if that's worth it,
> or if I should just elog(ERROR) and give up.

I think you may as well keep it the same as the other RI routines and
just elog() on SPI error. If SPI is broken, the trigger procedure is
gonna fail too.

I changed that, consolidated the error-reporting code, and fixed a couple
other little issues, notably:

* The generated query applied ONLY to the FK table but not the PK table.
I assume this was just an oversight.

* The query is now run using SPI_execp_current and selecting "current"
snapshot. Without this, we could fail in a serializable transaction
if someone else has already committed changes to either relation.
For example:

create pk and fk tables;

begin serializable xact;
insert into pk values(1);
insert into fk values(1);

begin;
insert into fk values(2);
commit;

alter table fk add foreign key ...;

The ALTER will not be blocked from acquiring exclusive lock, since
T2 already committed. But if we run the query in the serializable
snapshot, it won't see the violating row fk=2.

The old trigger-based check avoids this error because the scan loop uses
SnapshotNow to select live rows from the FK table. There is a dual race
condition where T2 deletes a row from the PK table. In current CVS tip
this will be detected and reported as a serialization failure, because
T1 won't be able to get SELECT FOR UPDATE lock on the deleted row. With
the proposed patch you'll instead see a "no such key" failure, which I
think is fine, even though it nominally violates serializability.

Comments? Can anyone else do a code review (Jan??)?

regards, tom lane