From: Peter Rabbitson Date: Mon, 9 May 2011 23:16:11 +0000 (+0200) Subject: Fix serious DBD::SQLite numeric datatype mismatch regression X-Git-Tag: v0.08192~5 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=632d1e0fcec8dd0b2ba1c2f9f616bf92aa22e2b3;p=dbsrgits%2FDBIx-Class.git Fix serious DBD::SQLite numeric datatype mismatch regression --- diff --git a/Changes b/Changes index 34e8f47..2a2c251 100644 --- a/Changes +++ b/Changes @@ -1,9 +1,10 @@ Revision history for DBIx::Class -0.08191 2011-05-02 00:45 (UTC) - No major changes since dev rel, see below for Changes + * Fixes + - Fix serious regression on SQLite, corrupting data when an alphanum + value does not correspond to a stale numeric datatype in colinfo -0.08190_01 2011-05-02 15:00 (UTC) +0.08191 2011-05-02 00:45 (UTC) (deleted from CPAN) * New Features / Changes - Add quote_names connection option. When set to true automatically sets quote_char and name_sep appropriate for your RDBMS diff --git a/lib/DBIx/Class/Storage/DBI/SQLite.pm b/lib/DBIx/Class/Storage/DBI/SQLite.pm index ecdc37d..6ca2cf5 100644 --- a/lib/DBIx/Class/Storage/DBI/SQLite.pm +++ b/lib/DBIx/Class/Storage/DBI/SQLite.pm @@ -6,6 +6,10 @@ use warnings; use base qw/DBIx::Class::Storage::DBI/; use mro 'c3'; +use DBIx::Class::Carp; +use Scalar::Util 'looks_like_number'; +use namespace::clean; + __PACKAGE__->sql_maker_class('DBIx::Class::SQLMaker::SQLite'); __PACKAGE__->sql_limit_dialect ('LimitOffset'); __PACKAGE__->sql_quote_char ('"'); @@ -93,6 +97,40 @@ sub bind_attribute_by_data_type { ; } +# DBD::SQLite (at least up to version 1.31 has a bug where it will +# non-fatally nummify a string value bound as an integer, resulting +# in insertions of '0' into supposed-to-be-numeric fields +# Since this can result in severe data inconsistency, remove the +# bind attr if such a sitation is detected +# +# FIXME - when a DBD::SQLite version is released that eventually fixes +# this sutiation (somehow) - no-op this override once a proper DBD +# version is detected +sub _dbi_attrs_for_bind { + my ($self, $ident, $bind) = @_; + my $bindattrs = $self->next::method($ident, $bind); + + for (0.. $#$bindattrs) { + if ( + defined $bindattrs->[$_] + and + defined $bind->[$_][1] + and + $bindattrs->[$_] eq DBI::SQL_INTEGER() + and + ! looks_like_number ($bind->[$_][1]) + ) { + carp_unique( sprintf ( + "Non-numeric value supplied for column '%s' despite the numeric datatype", + $bind->[$_][0]{dbic_colname} || "# $_" + ) ); + undef $bindattrs->[$_]; + } + } + + return $bindattrs; +} + =head2 connect_call_use_foreign_keys Used as: diff --git a/t/752sqlite.t b/t/752sqlite.t index 51541ac..31aacf6 100644 --- a/t/752sqlite.t +++ b/t/752sqlite.t @@ -3,6 +3,7 @@ use warnings; use Test::More; use Test::Exception; +use Test::Warn; use lib qw(t/lib); use DBICTest; @@ -26,6 +27,7 @@ $schema->txn_do(sub { } qr/rolling back inner transaction/, 'inner transaction rollback executed'; $ars->create({ name => 'in_outer_transaction2' }); }); + ok($ars->search({ name => 'in_outer_transaction' })->first, 'commit from outer transaction'); ok($ars->search({ name => 'in_outer_transaction2' })->first, @@ -36,6 +38,15 @@ is $ars->search({ name => 'in_inner_transaction_rolling_back' })->first, undef, 'rollback from inner transaction'; +# make sure the side-effects of RT#67581 do not result in data loss +my $row; +warnings_exist { $row = $ars->create ({ name => 'alpha rank', rank => 'abc' }) } + [qr/Non-numeric value supplied for column 'rank' despite the numeric datatype/], + 'proper warning on string insertion into an numeric column' +; +$row->discard_changes; +is ($row->rank, 'abc', 'proper rank inserted into database'); + done_testing; # vim:sts=2 sw=2: diff --git a/t/delete/related.t b/t/delete/related.t index 49cd88f..12bc43a 100644 --- a/t/delete/related.t +++ b/t/delete/related.t @@ -55,6 +55,8 @@ is ($cdrs->count, $total_cds -= 1, 'related + limit delete ok'); TODO: { local $TODO = 'delete_related is based on search_related which is based on search which does not understand object arguments'; + local $SIG{__WARN__} = sub {}; # trap the non-numeric warning, remove when the TODO is removed + my $cd2pr_count = $cd2pr_rs->count; $prod_cd->delete_related('cd_to_producer', { producer => $prod } ); is ($cd2pr_rs->count, $cd2pr_count -= 1, 'm2m link deleted succesfully');