X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=blobdiff_plain;f=lib%2FDBIx%2FClass%2FStorage%2FDBI%2FSQLite.pm;h=bd63417f7786f60e1ea5912e67aee3dc88a438a8;hb=f9b5239ac;hp=14c07d23742d478651b16d618d2e338fd78289d3;hpb=ad7c50fc26e1304855438776d88f4dd074d2fe05;p=dbsrgits%2FDBIx-Class.git diff --git a/lib/DBIx/Class/Storage/DBI/SQLite.pm b/lib/DBIx/Class/Storage/DBI/SQLite.pm index 14c07d2..bd63417 100644 --- a/lib/DBIx/Class/Storage/DBI/SQLite.pm +++ b/lib/DBIx/Class/Storage/DBI/SQLite.pm @@ -6,8 +6,8 @@ use warnings; use base qw/DBIx::Class::Storage::DBI/; use mro 'c3'; +use DBIx::Class::_Util 'modver_gt_or_eq'; use DBIx::Class::Carp; -use Scalar::Util 'looks_like_number'; use Try::Tiny; use namespace::clean; @@ -30,6 +30,47 @@ DBIx::Class::Storage::DBI::SQLite - Automatic primary key class for SQLite This class implements autoincrements for SQLite. +=head2 Known Issues + +=over + +=item RT79576 + + NOTE - This section applies to you only if ALL of these are true: + + * You are or were using DBD::SQLite with a version lesser than 1.38_01 + + * You are or were using DBIx::Class versions between 0.08191 and 0.08209 + (inclusive) or between 0.08240-TRIAL and 0.08242-TRIAL (also inclusive) + + * You use objects with overloaded stringification and are feeding them + to DBIC CRUD methods directly + +An unfortunate chain of events led to DBIx::Class silently hitting the problem +described in L. + +In order to trigger the bug condition one needs to supply B +bind value that is an object with overloaded stringification (nummification +is not relevant, only stringification is). When this is the case the internal +DBIx::Class call to C<< $sth->bind_param >> would be executed in a way that +triggers the above-mentioned DBD::SQLite bug. As a result all the logs and +tracers will contain the expected values, however SQLite will receive B +these bind positions being set to the value of the B supplied +stringifiable object. + +Even if you upgrade DBIx::Class (which works around the bug starting from +version 0.08210) you may still have corrupted/incorrect data in your database. +DBIx::Class will currently detect when this condition (more than one +stringifiable object in one CRUD call) is encountered and will issue a warning +pointing to this section. This warning will be removed 2 years from now, +around April 2015, You can disable it after you've audited your data by +setting the C environment variable. Note - the warning +is emited only once per callsite per process and only when the condition in +question is encountered. Thus it is very unlikey that your logsystem will be +flooded as a result of this. + +=back + =head1 METHODS =cut @@ -107,69 +148,74 @@ sub _ping { return undef unless $dbh->FETCH('Active'); return undef unless $dbh->ping; - # since we do not have access to sqlite3_get_autocommit(), do a trick - # to attempt to *safely* determine what state are we *actually* in. - # FIXME - # also using T::T here leads to bizarre leaks - will figure it out later - my $really_not_in_txn = do { - local $@; - - # older versions of DBD::SQLite do not properly detect multiline BEGIN/COMMIT - # statements to adjust their {AutoCommit} state. Hence use such a statement - # pair here as well, in order to escape from poking {AutoCommit} needlessly - # https://rt.cpan.org/Public/Bug/Display.html?id=80087 - eval { - # will fail instantly if already in a txn - $dbh->do("-- multiline\nBEGIN"); - $dbh->do("-- multiline\nCOMMIT"); - 1; - } or do { - ($@ =~ /transaction within a transaction/) - ? 0 - : undef - ; - }; - }; - my $ping_fail; - # if we were unable to determine this - we may very well be dead - if (not defined $really_not_in_txn) { - $ping_fail = 1; - } - # check the AC sync-state - elsif ($really_not_in_txn xor $dbh->{AutoCommit}) { - carp_unique (sprintf - 'Internal transaction state of handle %s (apparently %s a transaction) does not seem to ' - . 'match its AutoCommit attribute setting of %s - this is an indication of a ' - . 'potentially serious bug in your transaction handling logic', - $dbh, - $really_not_in_txn ? 'NOT in' : 'in', - $dbh->{AutoCommit} ? 'TRUE' : 'FALSE', - ); - - # it is too dangerous to execute anything else in this state - # assume everything works (safer - worst case scenario next statement throws) - return 1; - } - else { - # do the actual test - $ping_fail = ! try { $dbh->do('SELECT * FROM sqlite_master LIMIT 1'); 1 }; + # older DBD::SQLite does not properly synchronize commit state between + # the libsqlite and the $dbh + unless (defined $DBD::SQLite::__DBIC_TXN_SYNC_SANE__) { + $DBD::SQLite::__DBIC_TXN_SYNC_SANE__ = modver_gt_or_eq('DBD::SQLite', '1.38_02'); } - if ($ping_fail) { - # it is possible to have a proper "connection", and have "ping" return - # false anyway (e.g. corrupted file). In such cases DBD::SQLite still - # keeps the actual file handle open. We don't really want this to happen, - # so force-close the handle via DBI itself - # - local $@; # so that we do not clober the real error as set above - eval { $dbh->disconnect }; # if it fails - it fails - return undef # the actual RV of _ping() - } - else { - return 1; + # fallback to travesty + unless ($DBD::SQLite::__DBIC_TXN_SYNC_SANE__) { + # since we do not have access to sqlite3_get_autocommit(), do a trick + # to attempt to *safely* determine what state are we *actually* in. + # FIXME + # also using T::T here leads to bizarre leaks - will figure it out later + my $really_not_in_txn = do { + local $@; + + # older versions of DBD::SQLite do not properly detect multiline BEGIN/COMMIT + # statements to adjust their {AutoCommit} state. Hence use such a statement + # pair here as well, in order to escape from poking {AutoCommit} needlessly + # https://rt.cpan.org/Public/Bug/Display.html?id=80087 + eval { + # will fail instantly if already in a txn + $dbh->do("-- multiline\nBEGIN"); + $dbh->do("-- multiline\nCOMMIT"); + 1; + } or do { + ($@ =~ /transaction within a transaction/) + ? 0 + : undef + ; + }; + }; + + # if we were unable to determine this - we may very well be dead + if (not defined $really_not_in_txn) { + $ping_fail = 1; + } + # check the AC sync-state + elsif ($really_not_in_txn xor $dbh->{AutoCommit}) { + carp_unique (sprintf + 'Internal transaction state of handle %s (apparently %s a transaction) does not seem to ' + . 'match its AutoCommit attribute setting of %s - this is an indication of a ' + . 'potentially serious bug in your transaction handling logic', + $dbh, + $really_not_in_txn ? 'NOT in' : 'in', + $dbh->{AutoCommit} ? 'TRUE' : 'FALSE', + ); + + # it is too dangerous to execute anything else in this state + # assume everything works (safer - worst case scenario next statement throws) + return 1; + } } + + # do the actual test and return on no failure + ( $ping_fail ||= ! try { $dbh->do('SELECT * FROM sqlite_master LIMIT 1'); 1 } ) + or return 1; # the actual RV of _ping() + + # ping failed (or so it seems) - need to do some cleanup + # it is possible to have a proper "connection", and have "ping" return + # false anyway (e.g. corrupted file). In such cases DBD::SQLite still + # keeps the actual file handle open. We don't really want this to happen, + # so force-close the handle via DBI itself + # + local $@; # so that we do not clober the real error as set above + eval { $dbh->disconnect }; # if it fails - it fails + undef; # the actual RV of _ping() } sub deployment_statements { @@ -186,6 +232,10 @@ sub deployment_statements { $sqltargs->{producer_args}{sqlite_version} = $dver; } + $sqltargs->{quote_identifiers} + = !!$self->sql_maker->_quote_chars + if ! exists $sqltargs->{quote_identifiers}; + $self->next::method($schema, $type, $version, $dir, $sqltargs, @rest); } @@ -207,9 +257,17 @@ sub bind_attribute_by_data_type { # version is detected sub _dbi_attrs_for_bind { my ($self, $ident, $bind) = @_; + my $bindattrs = $self->next::method($ident, $bind); + # an attempt to detect former effects of RT#79576, bug itself present between + # 0.08191 and 0.08209 inclusive (fixed in 0.08210 and higher) + my $stringifiable = 0; + for (0.. $#$bindattrs) { + + $stringifiable++ if ( length ref $bind->[$_][1] and overload::Method($bind->[$_][1], '""') ); + if ( defined $bindattrs->[$_] and @@ -217,16 +275,24 @@ sub _dbi_attrs_for_bind { and $bindattrs->[$_] eq DBI::SQL_INTEGER() and - ! looks_like_number ($bind->[$_][1]) + $bind->[$_][1] !~ /^ [\+\-]? [0-9]+ (?: \. 0* )? $/x ) { carp_unique( sprintf ( - "Non-numeric value supplied for column '%s' despite the numeric datatype", + "Non-integer value supplied for column '%s' despite the integer datatype", $bind->[$_][0]{dbic_colname} || "# $_" ) ); undef $bindattrs->[$_]; } } + carp_unique( + 'POSSIBLE *PAST* DATA CORRUPTION detected - see ' + . 'DBIx::Class::Storage::DBI::SQLite/RT79576 or ' + . 'http://v.gd/DBIC_SQLite_RT79576 for further details or set ' + . '$ENV{DBIC_RT79576_NOWARN} to disable this warning. Trigger ' + . 'condition encountered' + ) if (!$ENV{DBIC_RT79576_NOWARN} and $stringifiable > 1); + return $bindattrs; }