Troubleshooting SPI_execp() failed in RI_FKey_cascade_del()

On Wed, 12 Jun 2002, Joshua b. Jore wrote:

I found the function in src/backend/utils/adt/ri_triggers.c. A previous
note in the archives says it might be an invalid SPI command/statement. So
how do I debug that? Which traces ought to be turned on?

Turn on query logging probably is the first step. A common cause of this
would be if there's a rule that might be rewriting the query to something
else (for cascade_del I'd guess that'd be a delete rule on the foreign
key table).

The following sql demonstrates the problem. What I'm getting at here is
cases where the rows in "b" are being altered instead of being deleted
consequently the delete to "a" shouldn't happen.

create table a (id int4 primary key);
create table b (id int4 references a on delete cascade);
create rule b0 as on delete to b do instead nothing;

insert into a values (1);
insert into b values (1);
delete from a;

Joshua b. Jore ; http://www.greentechnologist.org

On Wed, 12 Jun 2002, Joshua b. Jore wrote:

On Wed, 12 Jun 2002, Joshua b. Jore wrote:

The following sql demonstrates the problem. What I'm getting at here is
cases where the rows in "b" are being altered instead of being deleted
consequently the delete to "a" shouldn't happen.

create table a (id int4 primary key);
create table b (id int4 references a on delete cascade);
create rule b0 as on delete to b do instead nothing;

insert into a values (1);
insert into b values (1);
delete from a;

When you do the delete from a, the constraint will no longer
be satisifed. It throws the error to prevent the delete
from working. Maybe throwing a standard constraint violation
would be enough, but I'd worry that there'd be some other
return case that we should know about.

In general on <x> rules with on <x> action foreign key constraints
are a bad idea. I'd almost want to disallow the above entirely.

Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:

On Wed, 12 Jun 2002, Joshua b. Jore wrote:

The following sql demonstrates the problem. What I'm getting at here is
cases where the rows in "b" are being altered instead of being deleted
consequently the delete to "a" shouldn't happen.

When you do the delete from a, the constraint will no longer
be satisifed. It throws the error to prevent the delete
from working.

While I agree that some error should be thrown here, the actual behavior
seems all wrong. It looks to me like we're getting an error as a result
of an internal crosscheck that happens to be unhappy because of the
particular result that spi.c returns when a rule rewrites a DELETE to
"do instead nothing". This is so fragile --- it could break anytime
someone decides to clean up any of several routines. What's worse,
it will not detect interference from a rule that rewrites DELETEs in
any way less drastic than "do instead nothing", even if the rule has
the effect of suppressing the particular delete we need to do.

I think we are looking at another effect of the foreign-key
implementation being based on much higher-level operations than it
should be. Would it be feasible to tweak the SELECTs in these
RI triggers to extract CTIDs for the target rows, and then invoke
heap_delete without going through a DELETE command?

regards, tom lane

On Wed, 12 Jun 2002, Tom Lane wrote:

Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:

On Wed, 12 Jun 2002, Joshua b. Jore wrote:

The following sql demonstrates the problem. What I'm getting at here is
cases where the rows in "b" are being altered instead of being deleted
consequently the delete to "a" shouldn't happen.

When you do the delete from a, the constraint will no longer
be satisifed. It throws the error to prevent the delete
from working.

While I agree that some error should be thrown here, the actual behavior
seems all wrong. It looks to me like we're getting an error as a result
of an internal crosscheck that happens to be unhappy because of the
particular result that spi.c returns when a rule rewrites a DELETE to
"do instead nothing". This is so fragile --- it could break anytime
someone decides to clean up any of several routines. What's worse,
it will not detect interference from a rule that rewrites DELETEs in
any way less drastic than "do instead nothing", even if the rule has
the effect of suppressing the particular delete we need to do.

Yep, which is why I'd said that I'd want to disallow it (assuming
that fk uses DELETE). Of course that still wouldn't probably help
before delete triggers that returned NULL in some cases.

I think we are looking at another effect of the foreign-key
implementation being based on much higher-level operations than it
should be. Would it be feasible to tweak the SELECTs in these
RI triggers to extract CTIDs for the target rows, and then invoke
heap_delete without going through a DELETE command?

Well, we'd still need to do trigger firing for other foreign keys,
and possibly if users had logging triggers and such those should
fire (I'm not sure what SQL99 says, I have it at work, but not here).

I think my general complaint is that the error message is so inscrutable.
The actual SQL code was the result of two separate thoughts, implemented
on different days. The original 'on delete do' was actually 'instead
update .....' which on further reflection would never work since the the
constraints would have killed the transaction anyway. So I was wrong to
write that in the first place - I think it might have been nice to know
that prior to digging into the source code for
backend/utils/adt/ri_trigger.c and backend/executor/spi.c.

I suppose it's not actually that important or anything especially since
it's a "don't do that" sort of thing. I appreciated the kick in the pants
regardless.

Thanks much,
Joshua b. Jore ; http://www.greentechnologist.org

On Wed, 12 Jun 2002, Tom Lane wrote:

Stephan Szabo wrote:

On Wed, 12 Jun 2002, Joshua b. Jore wrote:

The following sql demonstrates the problem. What I'm getting at here is
cases where the rows in "b" are being altered instead of being deleted
consequently the delete to "a" shouldn't happen.

create table a (id int4 primary key);
create table b (id int4 references a on delete cascade);
create rule b0 as on delete to b do instead nothing;

insert into a values (1);
insert into b values (1);
delete from a;

When you do the delete from a, the constraint will no longer
be satisifed. It throws the error to prevent the delete
from working. Maybe throwing a standard constraint violation
would be enough, but I'd worry that there'd be some other
return case that we should know about.

In general on <x> rules with on <x> action foreign key constraints
are a bad idea. I'd almost want to disallow the above entirely.

Trying to protect RI against *every* feature in PostgreSQL
is dangerous IMHO. It might break useful administrative
possibilities. Actually, every RI constraint can be violated
with TRUNCATE. So should we disable TRUNCATE for tables that
have triggers or rules?

The referenced columns must be unique, protected by a unique
index. But yet it's still possible to drop that index later.
Should we prevent that too? If not, I can show you alot of
funny stuff possible now!

Rewrite rules are in general a mechanism to make views
updatable. They have originally been an idea to implement
an alternative to triggers, but that thesis didn't hold true.
And the instance level rules are long gone anyway.

Let's not get paranoid just because someone with alot of
PostgreSQL expertise can construct a schema that allows
RI breakage.

Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck@Yahoo.com #

On Thu, 13 Jun 2002, Jan Wieck wrote:

Stephan Szabo wrote:

On Wed, 12 Jun 2002, Joshua b. Jore wrote:

The following sql demonstrates the problem. What I'm getting at here is
cases where the rows in "b" are being altered instead of being deleted
consequently the delete to "a" shouldn't happen.

create table a (id int4 primary key);
create table b (id int4 references a on delete cascade);
create rule b0 as on delete to b do instead nothing;

insert into a values (1);
insert into b values (1);
delete from a;

When you do the delete from a, the constraint will no longer
be satisifed. It throws the error to prevent the delete
from working. Maybe throwing a standard constraint violation
would be enough, but I'd worry that there'd be some other
return case that we should know about.

In general on <x> rules with on <x> action foreign key constraints
are a bad idea. I'd almost want to disallow the above entirely.

Trying to protect RI against *every* feature in PostgreSQL
is dangerous IMHO. It might break useful administrative
possibilities. Actually, every RI constraint can be violated
with TRUNCATE. So should we disable TRUNCATE for tables that
have triggers or rules?

Isn't part of that (with TRUNCATE) on the todo list?

The referenced columns must be unique, protected by a unique
index. But yet it's still possible to drop that index later.
Should we prevent that too? If not, I can show you alot of
funny stuff possible now!

True. And there are other places that at least at one time (I haven't
tested recently) that somewhat rely on the brokenness of unique
constraints to work properly. I'd like to eventually see the
constraint triggers try to handle as many of those situations
as is feasible though.

Rewrite rules are in general a mechanism to make views
updatable. They have originally been an idea to implement
an alternative to triggers, but that thesis didn't hold true.
And the instance level rules are long gone anyway.

Let's not get paranoid just because someone with alot of
PostgreSQL expertise can construct a schema that allows
RI breakage.

True, but it's not obvious to someone that they are breaking
RI when they do the above in general. I think more people
are doing this and then wondering why it's breaking
than are doing it on purpose given the past couple of times
this has come up. I guess we need to decide at what level
we want to guarantee the constraint and how much work we're
willing to have the computer do to check. For example, a
select after the delete rather than the specific check on
SPI_execp would catch most cases of rule/trigger prevention
unless other after triggers were involved. But that requires another
query. Is it worth the extra query? I'm not sure. Is it
worth coming up with a way that catches other after triggers
on the real query being run in a rewrite case? Probably not,
since I don't see an obvious way of doing so.