| Subject: | Bizarre bug in multi-create on MySQL [PATCH/TEST] |
This took me a whole day to track down: in some circumstances repeated
multi-creates on a MySQL storage backend will cause data corruption
when the primary key in the related table is not explicitly set
(e.g. to undef).
I think the main culprit here is find_or_new_related in
DBIx::Class::Relationship::Base. It calls find_related with
not strictly illegal but at least useless arguments, maybe triggering
a bug further down the stack (I still wonder why this doesn't happen
with SQLite). Changing find_or_new_related to just pass on values
that are either unique or primary keys resp. not calling find_related
at all if no keys were given seems to fix this bug without altering
the intended behavior.
I've attached a testcase that demonstrates the bug and the fix
and a patch to the current trunk that can be applied if there are no
concerns about the fix (the testsuite still works after applying the patch).
Cheers,
Sebastian Willert
| Subject: | patch.diff |
Index: lib/DBIx/Class/Relationship/Base.pm
===================================================================
--- lib/DBIx/Class/Relationship/Base.pm (revision 3684)
+++ lib/DBIx/Class/Relationship/Base.pm (working copy)
@@ -288,7 +288,27 @@
sub find_or_new_related {
my $self = shift;
- return $self->find_related(@_) || $self->new_related(@_);
+ my $rel = shift;
+ my $attrs = shift;
+
+ $self->throw_exception( "find_or_new_related needs a hashref" )
+ unless ref $attrs eq 'HASH';
+
+ my $rel_src = $self->result_source->related_source($rel);
+ my @related_keys = map{ $rel_src->unique_constraint_columns($_) }
+ $rel_src->unique_constraint_names;
+
+ # Build primary and unique key conditions for find_related
+ my %keyed_attrs;
+ for my $key ( @related_keys ) {
+ $keyed_attrs{$key} = $attrs->{$key} if exists $attrs->{$key};
+ }
+
+ my $new;
+ $new = $self->find_related( $rel, \%keyed_attrs ) if %keyed_attrs;
+ $new ||= $self->new_related( $rel, $attrs );
+
+ return $new;
}
=head2 find_or_create_related
| Subject: | 96_multi_create_mysql.t |
#!/usr/bin/perl -w
use strict;
use warnings;
package MCTest::Schema::Related;
use base qw/DBIx::Class::Core/;
__PACKAGE__->table('multi_create_mysql_related');
__PACKAGE__->add_columns(
id => { data_type => "integer", is_auto_increment => 1, size => 11 },
);
__PACKAGE__->set_primary_key(qw/id/);
package MCTest::Schema::Main;
use base qw/DBIx::Class::Core/;
__PACKAGE__->table( 'multi_create_mysql_main' );
__PACKAGE__->add_columns(
id => { data_type => "integer", is_auto_increment => 1, size => 11 },
related => { data_type => "integer", size => 11 },
);
__PACKAGE__->set_primary_key( 'id' );
__PACKAGE__->belongs_to(
related => q/MCTest::Schema::Related/ => undef, { cascade_delete => 1 }
);
package MCTest::Schema;
use base qw/DBIx::Class::Schema/;
__PACKAGE__->load_classes(qw/Related Main/);
package main;
use Test::More;
my ($dsn, $user, $pass) = @ENV{map { "DBICTEST_MYSQL_${_}" } qw/DSN USER PASS/};
plan skip_all=> 'Set $ENV{DBICTEST_MYSQL_DSN}, _USER and _PASS to run this test'
unless $dsn && $user;
plan tests => 10;
sub init_schema {
my $schema = MCTest::Schema->connect(
$dsn, $user, $pass, { AutoCommit => 1 }
);
$schema->deploy({ add_drop_table => 1 });
return $schema;
}
sub fix_multi_create {
no strict 'refs';
no warnings 'redefine';
*{'DBIx::Class::Relationship::Base::find_or_new_related'} = sub{
my $self = shift;
my $rel = shift;
my $attrs = shift;
$self->throw_exception( "find_or_new_related needs a hashref" )
unless ref $attrs eq 'HASH';
my $rel_src = $self->result_source->related_source($rel);
my @related_keys = map{ $rel_src->unique_constraint_columns($_) }
$rel_src->unique_constraint_names;
# Build primary and unique key conditions for find_related
my %keyed_attrs;
for my $key ( @related_keys ) {
$keyed_attrs{$key} = $attrs->{$key} if exists $attrs->{$key};
}
my $new;
$new = $self->find_related( $rel, \%keyed_attrs ) if %keyed_attrs;
$new ||= $self->new_related( $rel, $attrs );
return $new;
};
Class::C3::reinitialize();
}
sub clear_table_content {
# delete_all doesn't work with corrupted tables so we have
# to use desperate measures to get rid of the data
my $schema = shift;
$schema->storage->dbh->do('truncate multi_create_mysql_main');
$schema->storage->dbh->do('truncate multi_create_mysql_related');
}
my $schema = init_schema();
my ( $tpl, $cnt ) = map{ $schema->resultset($_) } qw/Main Related/;
my $rel;
$tpl->create({ related => {} });
my $main = $tpl->first;
is( $main->id, 1, 'Single multi create (main)' );
is( $main->related->id, 1, 'Single multi create (related)' );
$tpl->create({ related => {} });
$main = $tpl->find(2);
is( $main->id, 2, 'Double multi create (main)' );
is( $main->related->id, 2, 'Double multi create (related)' );
clear_table_content( $schema );
$tpl->create({ related => {} }) for 1 .. 4;
is( $cnt->count, 4, 'Multi create in a loop' );
clear_table_content( $schema );
$tpl->create({ related => { id =>undef } }) for 1 .. 4;
is( $cnt->count, 4, 'Multi create with explicit id' );
clear_table_content( $schema );
$rel = $cnt->create({});
$tpl->create({ related => $rel->id });
$tpl->create({ related => {} }) for 1 .. 4;
is( $cnt->count, 5, 'Multi create with one exisitng entry' );
clear_table_content( $schema );
$rel = $cnt->create({});
$tpl->create({ related => $rel->id });
$rel = $cnt->create({});
$tpl->create({ related => $rel->id });
$tpl->create({ related => {} }) for 1 .. 4;
is( $cnt->count, 6, 'Multi create with two exisitng entries' );
fix_multi_create();
clear_table_content( $schema );
$tpl->create({ related => {} }) for 1 .. 4;
is( $cnt->count, 4, 'Multi create using fixed find_or_new_related' );
clear_table_content( $schema );
$tpl->create({ related => {} }) for 1 .. 4;
$tpl->create({ related => $cnt->first->id });
is( $cnt->count, 4, 'Multi create with retrival of an preexisting row' );
# clean up our mess
END {
my $dbh = $schema->storage->dbh;
$dbh->do("DROP TABLE multi_create_mysql_main") if $dbh;
$dbh->do("DROP TABLE multi_create_mysql_related") if $dbh;
}