[Dbix-class] bug in delete_all: CascadeActions::delete removes
parent before children
Noel Burton-Krahn
noel at burton-krahn.com
Sat Oct 25 00:48:02 BST 2008
Hi Peter,
The problem goes a little deeper: I applied this change to SVN, and
immediately saw the 60core.t unit test fail doing deep recursion.
Apparently this will break when there's a loop in the relationship
graph. So, to avoid the loop, I made a version of delete_all which
has loop protection. That fixes my unit test, but it breaks again in
60core.t because one of the search_related()->delete() calls produces
ambigious SQL. Sigh.
I attached my patch, which would reproduce this error when aplied to SVN:
make test
...
ok 15 - Correct artist too
DBIx::Class::Relationship::CascadeActions::delete(): DBI Exception:
DBD::SQLite::db prepare_cached failed: ambiguous column name: id2(1)
at dbdimp.c line 271 [for Statement "SELECT
artist_undirected_maps.id1, artist_undirected_maps.id2 FROM
artist_undirected_map me LEFT JOIN artist mapped_artists ON (
mapped_artists.artistid =3D me.id1 ) OR ( mapped_artists.artistid =3D
me.id2 ) LEFT JOIN artist_undirected_map artist_undirected_maps ON (
artist_undirected_maps.id1 =3D mapped_artists.artistid ) OR (
artist_undirected_maps.id2 =3D mapped_artists.artistid ) WHERE ( ( id1 =3D
? ) OR ( id2 =3D ? ) )"] at t/60core.t line 74
I'm not sure how to fix the ambiguous column name error though. Any though=
ts?
~Noel
On Fri, Oct 24, 2008 at 2:32 PM, Peter Rabbitson <rabbit+list at rabbit.us> wr=
ote:
> Noel Burton-Krahn wrote:
>> DBIx's cascading delete_all (in DBIx::Class::ResultSet) it broken,
>> because it deletes the parent table before it deletes the children.
>> The database will throw a referential integrity exception when the
>> parent is deleted before the children. I've attached a test program
>> below. Here's a fixed version in
>> DBIx-Class-0.08010/lib/DBIx/Class/Relationship/CascadeActions.pm:
>>
>> Regards,
>> Noel Burton-Krahn
>>
>> ###################################
>> # fixed DBIx/Class/Relationship/CascadeActions.pm in DBIx-Class-0.08010
>>
>> sub delete {
>> my ($self, @rest) =3D @_;
>>
>> # delete from tables that depend on me first
>> my $source =3D $self->result_source;
>> my %rels =3D map { $_ =3D> $source->relationship_info($_) }
>> $source->relationships;
>> my @cascade =3D grep { $rels{$_}{attrs}{cascade_delete} } keys %rels;
>> foreach my $rel (@cascade) {
>> $self->search_related($rel)->delete_all;
>> }
>>
>> # delete me
>> return $self->next::method(@rest) unless ref $self;
>> # I'm just ignoring this for class deletes because hell, the db shou=
ld
>> # be handling this anyway. Assuming we have joins we probably actual=
ly
>> # *could* do them, but I'd rather not.
>>
>> my $ret =3D $self->next::method(@rest);
>>
>> return $ret;
>> }
>>
>>
>>
>> # Test program
>> ###################################
>> #! /usr/bin/perl -w
>> =3Dhead1 NAME
>>
>> dbix_cascade_delete.t - reproduce DBIx's failure in delete_all
>>
>> =3Dhead1 DESCRIPTION
>>
>> DBIx::Class::ResultSet::delete_all fails in version 0.08010 because it
>> deletes the parent before the children
>>
>> =3Dhead1 AUTHOR
>>
>> Noel Burton-Krahn <noel at burton-krahn.com>
>>
>> =3Dcut
>>
>> use strict;
>> use warnings;
>>
>> #--------------------
>> package My::DBIx::Class;
>> use base qw/DBIx::Class/;
>>
>> __PACKAGE__->load_components(qw/PK::Auto Core/);
>>
>> use overload '""' =3D> 'dump';
>>
>> sub dump {
>> my($self) =3D shift;
>> return join(" ", map { "$_=3D" . $self->get_column($_) } $self->co=
lumns);
>> }
>>
>> #--------------------
>> package MySchema::Person;
>> use base qw/My::DBIx::Class/;
>> __PACKAGE__->table('person');
>> __PACKAGE__->add_columns(qw(person_id name));
>> __PACKAGE__->set_primary_key('person_id');
>> __PACKAGE__->has_many(address =3D> 'MySchema::Address', 'person_id');
>>
>> #--------------------
>> package MySchema::Address;
>> use base qw/My::DBIx::Class/;
>> __PACKAGE__->table('address');
>> __PACKAGE__->add_columns(qw(address_id person_id address));
>> __PACKAGE__->set_primary_key('address_id');
>> __PACKAGE__->belongs_to(person =3D> 'MySchema::Person', 'person_id');
>>
>> #--------------------
>> package MySchema;
>> use base qw/DBIx::Class::Schema/;
>>
>> __PACKAGE__->load_classes({
>> 'MySchema' =3D> [ qw(Person Address) ],
>> });
>>
>> #--------------------
>> package Test::DbixCascaseDelete;
>> use Test::More tests =3D> 16;
>>
>> # create a mysql database to test with
>> system(<<'EOS');
>> mysqladmin -f drop mytest >/dev/null 2>&1
>>
>> mysqladmin create mytest
>>
>> mysql mytest <<ESQL
>> create table person (
>> person_id INT NOT NULL AUTO_INCREMENT PRIMARY KEY
>> ,name VARCHAR(1024) NOT NULL
>> ) ENGINE=3DINNODB;
>>
>> create table address (
>> address_id INT NOT NULL AUTO_INCREMENT PRIMARY KEY
>> ,person_id INT NOT NULL
>> ,address VARCHAR(1024) NOT NULL
>> ,FOREIGN KEY (person_id) REFERENCES person (person_id)
>> ) ENGINE=3DINNODB;
>> ESQL
>>
>> #mysql mytest <<ESQL
>> #show tables;
>> #show create table person;
>> #show create table address;
>> #ESQL
>>
>> EOS
>> ;
>> is($?, 0, "create database");
>>
>> # connect
>> my $schema =3D MySchema->connect("dbi:mysql:mytest", 'script', 'tlby14')
>> or die("connect: $!");
>> ok($schema, "connect to db");
>>
>> #$schema->storage->debug(1);
>>
>> my $rs;
>> my $person;
>>
>> $person =3D $schema->resultset('Person')->create({ name =3D> 'fred'});
>> ok($person, "create Person: $person");
>>
>> $rs =3D $schema->resultset('Person')->search();
>> while( my $row =3D $rs->next() ) {
>> $person =3D $row;
>> }
>> ok($rs, "found Person: $person");
>>
>> my $address;
>> for my $i (1..3) {
>> $address =3D $schema->resultset('Address')->create({ person =3D> $=
person,
>> address =3D> "fred's address $i"});
>> ok($address, "create Address: $address");
>> }
>>
>> $rs =3D $schema->resultset('Address')->search({ person_id =3D>
>> $person->person_id });
>> while( my $row =3D $rs->next ) {
>> $address =3D $row;
>> ok($address, "found created Address: $address");
>> }
>>
>> ok($address->person, "address->person: " . $address->person->dump);
>>
>> $rs =3D $person->address_rs;
>> while( my $row =3D $rs->next ) {
>> $address =3D $row;
>> ok($address, "person->address: $address");
>> }
>>
>> $rs =3D $schema->resultset('Person')->search({ name =3D> 'fred'});
>>
>> $rs->delete_all;
>> ok(1, "delete_all");
>>
>> is($rs->count, 0, "Person really gone");
>>
>
> Please submit the lonely pieces of code above as a real diff against
> dbic trunk[1], so the actual set of changes is clearly visible
> (facilitates review and potential inclusion).
>
> Cheers
>
> [1]: svn co http://dev.catalyst.perl.org/repos/bast/DBIx-Class/0.08/trunk
>
> _______________________________________________
> List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
> IRC: irc.perl.org#dbix-class
> SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
> Searchable Archive: http://www.grokbase.com/group/dbix-class@lists.scsys.=
co.uk
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix_delete_all.patch
Type: text/x-diff
Size: 4750 bytes
Desc: not available
Url : http://lists.scsys.co.uk/pipermail/dbix-class/attachments/20081024/db=
43a88b/fix_delete_all.bin
More information about the DBIx-Class
mailing list