Many improvements of bigint handling across various DBD::SQLite versions
Peter Rabbitson [Tue, 8 Oct 2013 22:51:25 +0000 (00:51 +0200)]
This started as a patch by ilmari to bind all integers as SQL_BIGINT
as opposed to SQL_INTEGER. While this change itself worked fine, further
testing revealed we never actually bound 'bigint' column types in the
first place. When this was rectified all hell broke loose.

The current situation per DBD::SQLite version

1.29 ~ 1.33:  everything works fine, for large values DBD::SQLite loses
              precision on perl binaries with ivsize == 4 (just like integer
              math within perl itself)

1.35:         Nothing works *regardless* of ivsize. Binding a >32 bit value
              as either SQL_INTEGER or as SQL_BIGINT is impossible (an
              exception is thrown from within DBD::SQLite). DBIC simply strips
              the bindtype altogether and issues an "UPGRADE!" warning.

1.37 ~ 1.40:  Everything works, except for the annoying fact that DBD::SQLite
              issues a spurious 'datatype mismatch' warning when one tries
              to bind a >32 bit value with ivsize == 4. RT#76395
              Warning is silenced for the time being within the guts of DBIC.

On the SQL_INTEGER => SQL_BIGINT change: according to
http://www.sqlite.org/datatype3.html#storageclasses all numeric types are
dynamically allocated up to 8 bytes per individual value. Thus it should be
safe and non-wasteful to bind everything as SQL_BIGINT and have SQLite deal
with storage/comparisons however it deems correct.

Changes
lib/DBIx/Class.pm
lib/DBIx/Class/Storage/DBI/SQLite.pm
t/752sqlite.t

diff --git a/Changes b/Changes
index 031fddd..381ad0f 100644 (file)
--- a/Changes
+++ b/Changes
@@ -9,6 +9,10 @@ Revision history for DBIx::Class
           http://lists.scsys.co.uk/pipermail/dbix-class/2013-June/011374.html)
         - Setting quote_names propagates to SQL::Translator when producing
           SQLite DDL (it is one of the few producers *NOT* quoting by default)
+        - Fix incorrect binding of large integers on old versions of
+          DBD::SQLite (now DBIC simply always binds SQLite ints as BIGINT)
+        - Silence (harmless) warnings on recent DBD::SQLite versions, when
+          inserting/updating large integers on 32bit ivsize systems (RT#76395)
         - Back out self-cleaning from DBIx::Class::Carp for the time being
           (as a side effect fixes RT#86267)
         - Fix incorrect internal use of implicit list context in copy()
index 38fde7a..c26c9d6 100644 (file)
@@ -42,6 +42,8 @@ BEGIN {
     ,
 
     ASSERT_NO_INTERNAL_WANTARRAY => $ENV{DBIC_ASSERT_NO_INTERNAL_WANTARRAY} ? 1 : 0,
+
+    IV_SIZE => $Config{ivsize},
   };
 
   if ($] < 5.009_005) {
index 4ce820c..03fa8cf 100644 (file)
@@ -6,7 +6,7 @@ use warnings;
 use base qw/DBIx::Class::Storage::DBI/;
 use mro 'c3';
 
-use DBIx::Class::_Util 'modver_gt_or_eq';
+use DBIx::Class::_Util qw(modver_gt_or_eq sigwarn_silencer);
 use DBIx::Class::Carp;
 use Try::Tiny;
 use namespace::clean;
@@ -240,12 +240,37 @@ sub deployment_statements {
 }
 
 sub bind_attribute_by_data_type {
-  $_[1] =~ /^ (?: int(?:eger)? | (?:tiny|small|medium)int ) $/ix
-    ? DBI::SQL_INTEGER()
+
+  # According to http://www.sqlite.org/datatype3.html#storageclasses
+  # all numeric types are dynamically allocated up to 8 bytes per
+  # individual value
+  # Thus it should be safe and non-wasteful to bind everything as
+  # SQL_BIGINT and have SQLite deal with storage/comparisons however
+  # it deems correct
+  $_[1] =~ /^ (?: int(?:[1248]|eger)? | (?:tiny|small|medium|big)int ) $/ix
+    ? DBI::SQL_BIGINT()
     : undef
   ;
 }
 
+# FIXME - what the flying fuck... work around RT#76395
+# DBD::SQLite warns on binding >32 bit values with 32 bit IVs
+sub _dbh_execute {
+  if (DBIx::Class::_ENV_::IV_SIZE < 8) {
+
+    if (! defined $DBD::SQLite::__DBIC_CHECK_dbd_mishandles_bound_BIGINT) {
+      $DBD::SQLite::__DBIC_CHECK_dbd_mishandles_bound_BIGINT = (
+        modver_gt_or_eq('DBD::SQLite', '1.37')
+      ) ? 1 : 0;
+    }
+
+    local $SIG{__WARN__} = sigwarn_silencer( qr/datatype mismatch/ )
+      if $DBD::SQLite::__DBIC_CHECK_dbd_mishandles_bound_BIGINT;
+  }
+
+  shift->next::method(@_);
+}
+
 # DBD::SQLite (at least up to version 1.31 has a bug where it will
 # non-fatally numify a string value bound as an integer, resulting
 # in insertions of '0' into supposed-to-be-numeric fields
@@ -260,6 +285,15 @@ sub _dbi_attrs_for_bind {
 
   my $bindattrs = $self->next::method($ident, $bind);
 
+  # somewhere between 1.33 and 1.37 things went horribly wrong
+  if (! defined $DBD::SQLite::__DBIC_CHECK_dbd_can_bind_bigint_values) {
+    $DBD::SQLite::__DBIC_CHECK_dbd_can_bind_bigint_values = (
+      modver_gt_or_eq('DBD::SQLite', '1.34')
+        and
+      ! modver_gt_or_eq('DBD::SQLite', '1.37')
+    ) ? 0 : 1;
+  }
+
   # 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;
@@ -276,14 +310,34 @@ sub _dbi_attrs_for_bind {
       grep { $bindattrs->[$i] eq $_ } (
         DBI::SQL_INTEGER(), DBI::SQL_TINYINT(), DBI::SQL_SMALLINT(), DBI::SQL_BIGINT()
       )
-        and
-      $bind->[$i][1] !~ /^ [\+\-]? [0-9]+ (?: \. 0* )? $/x
     ) {
-      carp_unique( sprintf (
-        "Non-integer value supplied for column '%s' despite the integer datatype",
-        $bind->[$i][0]{dbic_colname} || "# $i"
-      ) );
-      undef $bindattrs->[$i];
+      if ( $bind->[$i][1] !~ /^ [\+\-]? [0-9]+ (?: \. 0* )? $/x ) {
+        carp_unique( sprintf (
+          "Non-integer value supplied for column '%s' despite the integer datatype",
+          $bind->[$i][0]{dbic_colname} || "# $i"
+        ) );
+        undef $bindattrs->[$i];
+      }
+      elsif (
+        ! $DBD::SQLite::__DBIC_CHECK_dbd_can_bind_bigint_values
+          and
+        # unsigned 32 bit ints have a range of −2,147,483,648 to 2,147,483,647
+        # alternatively expressed as the hexadecimal numbers below
+        # the comparison math will come out right regardless of ivsize, since
+        # we are operating within 31 bits
+        # P.S. 31 because one bit is lost for the sign
+        ($bind->[$i][1] > 0x7fff_ffff or $bind->[$i][1] < -0x8000_0000)
+      ) {
+        carp_unique( sprintf (
+          "An integer value occupying more than 32 bits was supplied for column '%s' "
+        . 'which your version of DBD::SQLite (%s) can not bind properly so DBIC '
+        . 'will treat it as a string instead, consider upgrading to at least '
+        . 'DBD::SQLite version 1.37',
+          $bind->[$i][0]{dbic_colname} || "# $i",
+          DBD::SQLite->VERSION,
+        ) );
+        undef $bindattrs->[$i];
+      }
     }
   }
 
index b273d97..8882846 100644 (file)
@@ -5,7 +5,6 @@ use Test::More;
 use Test::Exception;
 use Test::Warn;
 use Time::HiRes 'time';
-use Config;
 
 use lib qw(t/lib);
 use DBICTest;
@@ -150,32 +149,81 @@ $schema->storage->dbh_do(sub {
 
 # test upper/lower boundaries for sqlite and some values inbetween
 # range is -(2**63) .. 2**63 - 1
-SKIP: {
-  skip 'This perl does not seem to have 64bit int support - DBI roundtrip of large int will fail with DBD::SQLite < 1.37', 1
-    if ($Config{ivsize} < 8 and ! modver_gt_or_eq('DBD::SQLite', '1.37') );
-
-  for my $bi (qw/
-    -9223372036854775808
-    -9223372036854775807
-    -8694837494948124658
-    -6848440844435891639
-    -5664812265578554454
-    -5380388020020483213
-    -2564279463598428141
-    2442753333597784273
-    4790993557925631491
-    6773854980030157393
-    7627910776496326154
-    8297530189347439311
-    9223372036854775806
-    9223372036854775807
-  /) {
+for my $bi ( qw(
+  -9223372036854775808
+  -9223372036854775807
+  -8694837494948124658
+  -6848440844435891639
+  -5664812265578554454
+  -5380388020020483213
+  -2564279463598428141
+  2442753333597784273
+  4790993557925631491
+  6773854980030157393
+  7627910776496326154
+  8297530189347439311
+  9223372036854775806
+  9223372036854775807
+
+  4294967295
+  4294967296
+
+  -4294967296
+  -4294967295
+  -4294967294
+
+  -2147483649
+  -2147483648
+  -2147483647
+  -2147483646
+
+  2147483646
+  2147483647
+),
+  # these values cause exceptions even with all workarounds in place on these
+  # fucked DBD::SQLite versions *regardless* of ivsize >.<
+  ( modver_gt_or_eq('DBD::SQLite', '1.34') and ! modver_gt_or_eq('DBD::SQLite', '1.37') )
+    ? ()
+    : ( '2147483648', '2147483649' )
+) {
+  # unsigned 32 bit ints have a range of −2,147,483,648 to 2,147,483,647
+  # alternatively expressed as the hexadecimal numbers below
+  # the comparison math will come out right regardless of ivsize, since
+  # we are operating within 31 bits
+  # P.S. 31 because one bit is lost for the sign
+  my $v_bits = ($bi > 0x7fff_ffff || $bi < -0x8000_0000) ? 64 : 32;
+
+  my $v_desc = sprintf '%s (%d bit signed int)', $bi, $v_bits;
+
+  my $w;
+  lives_ok {
+    local $SIG{__WARN__} = sigwarn_silencer( qr/datatype mismatch/ );
     $row = $schema->resultset('BigIntArtist')->create({ bigint => $bi });
-    is ($row->bigint, $bi, "value in object correct ($bi)");
+  } "Insering value $bi ($v_desc)" or next;
 
-    $row->discard_changes;
-    is ($row->bigint, $bi, "value in database correct ($bi)");
-  }
+  is ($w, undef, 'No mismatch warning on bigints' );
+
+  # explicitly using eq, to make sure we did not nummify the argument
+  # which can be an issue on 32 bit ivsize
+  cmp_ok ($row->bigint, 'eq', $bi, "value in object correct ($v_desc)");
+
+  $row->discard_changes;
+
+  cmp_ok (
+    $row->bigint,
+
+    # the test will not pass an == if we are running under 32 bit ivsize
+    # use 'eq' on the numified (and possibly "scientificied") returned value
+    DBIx::Class::_ENV_::IV_SIZE < 8 ? 'eq' : '==',
+
+    # in 1.37 DBD::SQLite switched to proper losless representation of bigints
+    # regardless of ivize
+    # before this use 'eq' (from above) on the numified (and possibly
+    # "scientificied") returned value
+    (DBIx::Class::_ENV_::IV_SIZE < 8 and ! modver_gt_or_eq('DBD::SQLite', '1.37')) ? $bi+0 : $bi,
+
+    "value in database correct ($v_desc)"
+  );
 }
 
 done_testing;