[Dbix-class] bug in delete_all: CascadeActions::delete removes parent before children

[Peter Rabbitson]

Noel Burton-Krahn wrote:
> On Sun, Oct 26, 2008 at 1:19 AM, Peter Rabbitson <rabbit+list at rabbit.us> wrote:
>> 1) You call delete_all
>> 2) You traverse the entire relationship chain, and start calling delete
>> on the resulting three backwards, depth first.
>> 3) By the time you get to the main row that you wanted to delete_all in
>> the first place, it runs a trigger which is supposed to run some
>> statistics on the rows the ref_integrity was supposed to delete - epic
>> fail: they are not there anymore.
> 
> I don't agree with #3:  I do my business logic (in your example, doing
> statistics) in Perl, not database triggers.  That's one thing ORM is
> good for.

Neither do I, but for someone who does DBIC all of a sudden becomes a
non-starter.

> My example blows up because it has referential integrity checking, but
> no "ON DELETE CASCADE" triggers.  I'd prefer the ORM to handle
> cascading deletes, which is what I was hoping for with delete_all.

You can not have a referential constraint without ON UPDATE/ON DELETE
triggers. Refer to your RDBMS manual to find out which trigger is used
by default if one is not supplied explicitly (usually RESTRICT).

> Unfortunately, that can't work the way it's current;y implemented.
> So, I wonder why keep the current implementation of delete_all at all?

Because delete() on a resultset calls DELETE FROM <table> WHERE
<sql-abstract condition>, whereas delete_all() retrieves all the rows in
the resultset and then calls delete() on the row objects _one_by_one_.
This has two benefits:

1) Your business logic defined in Perl will actually run, since it is
most likely hooked up in an overloaded row-level delete(). If you do
$rs->delete, the row-level delete()s never get called, so again - no
statistics for you.

2) Most (if not all) databases do not support complex WHERE conditions
with joins for the DELETE call. So if the condition of your current $rs
is moderately complex you can not call ->delete on it and expect things
to work. Current trunk actually warns you of the fact.

All-in-all this is not a bug, but totally expected and _desired_
behavior. Fix your constraints.

> If you construct a sufficiently complex select_related clause, DBIx
> will construct ambiguous SQL.  Take a look at the SQL that DBIx
> generates:  It does not uniquely qualify the selected columns.  Here
> was the SQL that failed, which you'll see if you run my test in my
> patch:
> 
> SELECT artist_undirected_maps.id1, artist_undirected_maps.id2
> FROM artist_undirected_map me
> LEFT JOIN artist mapped_artists ON (
> mapped_artists.artistid = me.id1 ) OR ( mapped_artists.artistid =
> me.id2 )
> LEFT JOIN artist_undirected_map artist_undirected_maps ON (
> artist_undirected_maps.id1 = mapped_artists.artistid ) OR (
> artist_undirected_maps.id2 = mapped_artists.artistid )
> WHERE ( ( id1 = ? ) OR ( id2 = ? ) )
> 
> The id1 and id2 in the "where" clause should have been uniquely
> qualified like me.id1 and me.id2.
> 

I applied your patch to latest trunk, and this is what I get. Please
point out the problem:

rabbit at Thesaurus:~/devel/dbic/delete_rambling$ DBIC_TRACE=1 prove -Ilib
-It/lib t/100delete_all_cascades.t -v
t/100delete_all_cascades....1..16
ok 1 - create database
ok 2 - connect to db
INSERT INTO person (name) VALUES (?): 'fred'
ok 3 - create Person: id=1 name=fred
SELECT me.id, me.name FROM person me:
ok 4 - found Person: id=1 name=fred
INSERT INTO address (address, person_id) VALUES (?, ?): 'fred's address
1', '1'
ok 5 - create Address: id=1 person_id=1 address=fred's address 1
INSERT INTO address (address, person_id) VALUES (?, ?): 'fred's address
2', '1'
ok 6 - create Address: id=2 person_id=1 address=fred's address 2
INSERT INTO address (address, person_id) VALUES (?, ?): 'fred's address
3', '1'
ok 7 - create Address: id=3 person_id=1 address=fred's address 3
SELECT me.id, me.person_id, me.address FROM address me:
ok 8 - found created Address: id=1 person_id=1 address=fred's address 1
ok 9 - found created Address: id=2 person_id=1 address=fred's address 2
ok 10 - found created Address: id=3 person_id=1 address=fred's address 3
SELECT me.id, me.name FROM person me WHERE ( ( ( me.id = ? ) ) ): '1'
ok 11 - address->person: id=1 name=fred
SELECT me.id, me.person_id, me.address FROM address me WHERE (
me.person_id = ? ): '1'
ok 12 - person->address: id=1 person_id=1 address=fred's address 1
ok 13 - person->address: id=2 person_id=1 address=fred's address 2
ok 14 - person->address: id=3 person_id=1 address=fred's address 3
SELECT address.id, address.person_id, address.address FROM person me
LEFT JOIN address address ON ( address.person_id = me.id ) WHERE ( name
= ? ): 'fred'
DELETE FROM address WHERE ( id = ? ): '1'
DELETE FROM address WHERE ( id = ? ): '2'
DELETE FROM address WHERE ( id = ? ): '3'
SELECT me.id, me.name FROM person me WHERE ( name = ? ): 'fred'
DELETE FROM person WHERE ( id = ? ): '1'
SELECT me.id, me.person_id, me.address FROM address me WHERE (
me.person_id = ? ): '1'
ok 15 - delete_all
SELECT COUNT( * ) FROM person me WHERE ( name = ? ): 'fred'
ok 16 - Person really gone
ok
All tests successful.
Files=1, Tests=16,  0 wallclock secs ( 0.19 cusr +  0.02 csys =  0.21 CPU)
rabbit at Thesaurus:~/devel/dbic/delete_rambling$


Cheers