Cascades Failing

On Tue, 16 Aug 2005, Jake Stride wrote:

I seem to be having some problems with cascading updates, I seem to
remember that this worked in the database in 7.4 but seems to fail in 8,
can anyone give me any pointer please as I seem to be able to find
anything about this online.

I think this is similar to the issue recently reported on -bugs. My
theory there was that trigger timing changes between 7.4 and 8.0 seems to
have caused the sequence of checks inside the trigger manager and triggers
that prevented intermediate states from being checked to become broken.

Jake Stride wrote:

I seem to be having some problems with cascading updates, I seem to
remember that this worked in the database in 7.4 but seems to fail in 8,
can anyone give me any pointer please as I seem to be able to find
anything about this online.

No when I try to do an update I get the following error:

update users set username='new' where username='old';
ERROR: insert or update on table "company" violates foreign key
constraint "$5"
DETAIL: Key (alteredby)=(old) is not present in table "users".
CONTEXT: SQL statement "UPDATE ONLY "public"."company" SET "assigned" =
$1 WHERE "assigned" = $2"

surely this should not fail because of the 'ON UPDATE CASCADE'?

Hmm - I can reproduce it on 8.0.2 on my Mac laptop, but the following
works fine on version 7.4.7 which I also have here.

--
-- Begin test
--
CREATE TABLE foo (a int, b text, PRIMARY KEY (a));
CREATE TABLE bar (
x int,
y int NOT NULL REFERENCES foo(a) ON UPDATE CASCADE,
z int NOT NULL REFERENCES foo(a) ON UPDATE CASCADE
);

INSERT INTO foo VALUES (1,'aaa');
INSERT INTO foo VALUES (2,'bbb');
INSERT INTO bar VALUES (101, 1, 1);
INSERT INTO bar VALUES (102, 2, 2);

UPDATE foo SET a=3 WHERE a=1;
--
-- End test
--

There is something similar mentioned on the bugs list - "Cascading
updates run seperately". Does this look like your problem?

--
Richard Huxton
Archonet Ltd

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

I think this is similar to the issue recently reported on -bugs. My
theory there was that trigger timing changes between 7.4 and 8.0 seems to
have caused the sequence of checks inside the trigger manager and triggers
that prevented intermediate states from being checked to become broken.

Just looking at the example, I think that the issue is that we fire a
trigger for one of the FK constraints, it does an UPDATE to fix the
constraint it knows about, and then on the way out of that UPDATE
statement, check triggers for all of the FK constraints are executed
and the ones that haven't been fixed yet are unhappy. (The failure
occurs because two independent updates are needed on the same row of
the referencing table, and only one has been done yet.) So the problem
comes directly from the fact that FK triggers can fire at the ends of
nested statements, rather than only at the outer level as they did
before.

This suggests that we need a way to prevent immediate execution of
freshly queued triggers at the end of a command fired by an FK trigger.
If we could move them to the end of the trigger queue that the FK
operation itself is in, things would work reasonably well I think.

regards, tom lane

[ redirected to -hackers ]

I wrote:

This suggests that we need a way to prevent immediate execution of
freshly queued triggers at the end of a command fired by an FK trigger.
If we could move them to the end of the trigger queue that the FK
operation itself is in, things would work reasonably well I think.

After a quick look through the code, it seems like the way to do this
is to add an extra bool parameter "nest_triggers" to _SPI_pquery, which
when false would simply suppress its calls to AfterTriggerBeginQuery
and AfterTriggerEndQuery --- thus causing any queued triggers to be
queued in the same trigger list the FK is in. We'd then expose this
parameter (only) via SPI_execute_snapshot, which is intended only for
RI trigger use anyway.

I think this would take some generalization of afterTriggerInvokeEvents,
which now might or might not find the target rel in the EState it's
passed, but otherwise it doesn't seem too invasive. Thoughts?

regards, tom lane

On Tue, 16 Aug 2005, Tom Lane wrote:

[ redirected to -hackers ]

I wrote:

This suggests that we need a way to prevent immediate execution of
freshly queued triggers at the end of a command fired by an FK trigger.
If we could move them to the end of the trigger queue that the FK
operation itself is in, things would work reasonably well I think.

After a quick look through the code, it seems like the way to do this
is to add an extra bool parameter "nest_triggers" to _SPI_pquery, which
when false would simply suppress its calls to AfterTriggerBeginQuery
and AfterTriggerEndQuery --- thus causing any queued triggers to be
queued in the same trigger list the FK is in. We'd then expose this
parameter (only) via SPI_execute_snapshot, which is intended only for
RI trigger use anyway.

This seems right to me. I'd thought that SQL wanted the user triggers to
be run after the updating directly, but reading it again, SQL03 at least
seems to just talk about adding state changes for after triggers to the
current trigger context AFAICS which means that the above seems to be what
is requested by the spec in general.

I think this would take some generalization of afterTriggerInvokeEvents,
which now might or might not find the target rel in the EState it's
passed, but otherwise it doesn't seem too invasive. Thoughts?

That doesn't seem too bad really, looking at afterTriggerInvokeEvents it
doesn't look like it'd be that much work to change it to handle that case.
I can put a patch together to see what it looks like.

On Tuesday 16 August 2005 09:17, Stephan Szabo wrote:

On Tue, 16 Aug 2005, Tom Lane wrote:

[ redirected to -hackers ]

I wrote:

This suggests that we need a way to prevent immediate execution of
freshly queued triggers at the end of a command fired by an FK trigger.
If we could move them to the end of the trigger queue that the FK
operation itself is in, things would work reasonably well I think.

After a quick look through the code, it seems like the way to do this
is to add an extra bool parameter "nest_triggers" to _SPI_pquery, which
when false would simply suppress its calls to AfterTriggerBeginQuery
and AfterTriggerEndQuery --- thus causing any queued triggers to be
queued in the same trigger list the FK is in. We'd then expose this
parameter (only) via SPI_execute_snapshot, which is intended only for
RI trigger use anyway.

This seems right to me. I'd thought that SQL wanted the user triggers to
be run after the updating directly, but reading it again, SQL03 at least
seems to just talk about adding state changes for after triggers to the
current trigger context AFAICS which means that the above seems to be what
is requested by the spec in general.

I think this would take some generalization of afterTriggerInvokeEvents,
which now might or might not find the target rel in the EState it's
passed, but otherwise it doesn't seem too invasive. Thoughts?

That doesn't seem too bad really, looking at afterTriggerInvokeEvents it
doesn't look like it'd be that much work to change it to handle that case.
I can put a patch together to see what it looks like.

I have a realworld test case of delete cascade (approx 90 cascaded tables,
some more than 8 levels deep) failing on 8.0.3 (and 8.1) , this is one of a
few issues that is preventing me from upgrading a couple of 7.4 boxen to 8.x,
if you need testers for this patch, please let me know and I'll be glad to
try it out and see if it solves the cascade problems I am experiencing.

---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

--
Darcy Buskermolen
Wavefire Technologies Corp.

http://www.wavefire.com
ph: 250.717.0200
fx: 250.763.1759

On Tue, 16 Aug 2005, Stephan Szabo wrote:

On Tue, 16 Aug 2005, Tom Lane wrote:

I think this would take some generalization of afterTriggerInvokeEvents,
which now might or might not find the target rel in the EState it's
passed, but otherwise it doesn't seem too invasive. Thoughts?

That doesn't seem too bad really, looking at afterTriggerInvokeEvents it
doesn't look like it'd be that much work to change it to handle that case.
I can put a patch together to see what it looks like.

I did some work on this, and I'm getting a couple of other failures from
other parts of the foreign key regression test (specifically an error
that is no longer erroring in a multi-column on update set default). I'm
going to need to look more closely to see if I can figure out why.

On Wed, 17 Aug 2005, Stephan Szabo wrote:

On Tue, 16 Aug 2005, Stephan Szabo wrote:

On Tue, 16 Aug 2005, Tom Lane wrote:

I think this would take some generalization of afterTriggerInvokeEvents,
which now might or might not find the target rel in the EState it's
passed, but otherwise it doesn't seem too invasive. Thoughts?

That doesn't seem too bad really, looking at afterTriggerInvokeEvents it
doesn't look like it'd be that much work to change it to handle that case.
I can put a patch together to see what it looks like.

I did some work on this, and I'm getting a couple of other failures from
other parts of the foreign key regression test (specifically an error
that is no longer erroring in a multi-column on update set default). I'm
going to need to look more closely to see if I can figure out why.

I think I see at least part of what's going on. It looks to me that
events are being added, but not fired because they weren't
marked. The event sequence seems to be:

after trigger begin query
add events for the actual statement
after trigger end query
fire trigger
add events for the triggered statement
finish trigger
skip event added for triggered statement because it's not marked.

Is the correct answer to continue marking and running the triggers until
there are no immediate triggers left to run for this case?

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

Is the correct answer to continue marking and running the triggers until
there are no immediate triggers left to run for this case?

Hmm ... my recollection is that we put in the concept of marking because
we needed it for correct behavior in some cases. I don't recall exactly
why though.

regards, tom lane

On Fri, 19 Aug 2005, Tom Lane wrote:

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

Is the correct answer to continue marking and running the triggers until
there are no immediate triggers left to run for this case?

Hmm ... my recollection is that we put in the concept of marking because
we needed it for correct behavior in some cases. I don't recall exactly
why though.

The comment there talks about someone doing a set constraints immediate
inside a trigger function.

/*
* Process all immediate-mode triggers queued by the query, and
move
* the deferred ones to the main list of deferred events.
*
* Notice that we decide which ones will be fired, and put the
deferred
* ones on the main list, before anything is actually fired. This
* ensures reasonably sane behavior if a trigger function does
* SET CONSTRAINTS ... IMMEDIATE: all events we have decided to
defer
* will be available for it to fire.
*
* If we find no firable events, we don't have to increment
firing_counter.
*/

I think we were worried about it either skipping events or potentially
running events twice in that case, but I don't remember exactly either.

I'm not sure that looping would affect that though, it'd be basically
mark (0)
increment firing id (0->1)
run triggers using the old id (0)
- if the set constraints immediate was run here, it'd mark using the
- incremented id (hopefully incrementing again - will need to check) and
- run using that id (1->2) and (1)
mark (2)
increment firing id (2->3)
run triggers using (2)

There might be some other reason that's not enshrined in the comment
though.

On Fri, 19 Aug 2005, Stephan Szabo wrote:

On Fri, 19 Aug 2005, Tom Lane wrote:

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

Is the correct answer to continue marking and running the triggers until
there are no immediate triggers left to run for this case?

Hmm ... my recollection is that we put in the concept of marking because
we needed it for correct behavior in some cases. I don't recall exactly
why though.

Hmm, there's an issue with before triggers as well. We add the checks for
the updates to the end of the current statement's queue and shouldn't run
them until all the cascaded updates are done. However, if a before on
update trigger of the fk side also updates an fk row that is in the middle
of a series of these updates with a direct update statement, that
statement's check will happen inside the before trigger, which will fail.
It's not necessarily a triggered data change violation if the change
happens to not change the key values or sets them to what the have already
or will become. We could get around this by somehow inheriting the state
of our direct parent trigger (whether or not it was in a new query), but
that seems like it'd break other cases because the triggers would line up
in the pre-8.0 sequence in that case. Or, is it correct to fail in this
case because the statement is trying to update in a new query to a set of
invalid keys?

On Wed, Aug 17, 2005 at 08:53:28AM -0700, Darcy Buskermolen wrote:

I have a realworld test case of delete cascade (approx 90 cascaded tables,
some more than 8 levels deep) failing on 8.0.3 (and 8.1) , this is one of a
few issues that is preventing me from upgrading a couple of 7.4 boxen to 8.x,
if you need testers for this patch, please let me know and I'll be glad to
try it out and see if it solves the cascade problems I am experiencing.

Is this something we can put into the regression tests?
--
Jim C. Nasby, Sr. Engineering Consultant jnasby@pervasive.com
Pervasive Software http://pervasive.com 512-569-9461