From: Peter Rabbitson <ribasushi@cpan.org>
Date: Tue, 8 Jul 2014 04:58:03 +0000 (+0200)
Subject: Revert optimistic and sloppy changes from 3705e3b2 - DBI does not always DTRT
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=7cbd6cbd07bd62b29dfa60b361c774626b06e967;p=dbsrgits%2FDBIx-Class-Historic.git

Revert optimistic and sloppy changes from 3705e3b2 - DBI does not always DTRT

In addition I rebroke the issue from RT#79576, ffs man pay attention

<ribasushi> general question - does DBI guarantee that objects with stringification overload will get their stringification called
<ribasushi> or is this up to the DBD (and thus will vary)
<mje> ribasushi, I had an issue in DBD::ODBC ages ago with this - looking for it now
<mje>  rt 78838 - bind_param does not correctly stringify blessed objects when connected to MS SQL Server, magic was not being applied in DBD::ODBC case
<mje> so I think the answer to your question is DBI does not but DBDs should if they are written correctly
<ribasushi> I wonder why DBI does not
<timbunce_> ribasushi: no explicit ‘guarantee’ but I think any place it doesn’t is probably a bug.
<ribasushi> timbunce_: given mje had to do magic in his DBD, I suspect DBI has a "hole" somewhere then no?
<ribasushi> basically I was looking into removing the explicit stringifications in DBIC, hence the question
<ribasushi> if there is consensus that DBI ought to do it all on its own, we could test for it, fix it up, and then I disable the checks when I detect a sufficiently advanced DBI.pm
<timbunce> yes, ribasushi: “… we could test for it, fix it up, and then I disable the checks when I detect a sufficiently advanced DBI.pm or something”
<timbunce> :)
<ribasushi> nod ;)
---

diff --git a/Changes b/Changes
index 3adf7c7..0b56392 100644
--- a/Changes
+++ b/Changes
@@ -40,8 +40,6 @@ Revision history for DBIx::Class
     * Misc
         - IFF DBIC_TRACE output defaults to STDERR we now silence the possible
           wide-char warnings if the trace happens to contain unicode
-        - Stop explicitly stringifying objects before passing them to DBI,
-          instead assume that the DBI/DBD combo will take care of things
         - Remove ::ResultSource::resolve_condition - the underlying machinery
           is in flux, and the method has had a deprecation warning for 5 years
 
diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm
index 728d2b4..0e579e9 100644
--- a/lib/DBIx/Class/Storage/DBI.pm
+++ b/lib/DBIx/Class/Storage/DBI.pm
@@ -1895,9 +1895,16 @@ sub _bind_sth_params {
       );
     }
     else {
+      # FIXME SUBOPTIMAL - DBI needs fixing to always stringify regardless of DBD
+      my $v = ( length ref $bind->[$i][1] and is_plain_value $bind->[$i][1] )
+        ? "$bind->[$i][1]"
+        : $bind->[$i][1]
+      ;
+
       $sth->bind_param(
         $i + 1,
-        $bind->[$i][1],
+        # The temp-var is CRUCIAL - DO NOT REMOVE IT, breaks older DBD::SQLite RT#79576
+        $v,
         $bind_attrs->[$i],
       );
     }
@@ -2036,6 +2043,16 @@ sub insert_bulk {
 
   my @col_range = (0..$#$cols);
 
+  # FIXME SUBOPTIMAL - DBI needs fixing to always stringify regardless of DBD
+  # For the time being forcibly stringify whatever is stringifiable
+  # ResultSet::populate() hands us a copy - safe to mangle
+  for my $r (0 .. $#$data) {
+    for my $c (0 .. $#{$data->[$r]}) {
+      $data->[$r][$c] = "$data->[$r][$c]"
+        if ( length ref $data->[$r][$c] and is_plain_value $data->[$r][$c] );
+    }
+  }
+
   my $colinfos = $source->columns_info($cols);
 
   local $self->{_autoinc_supplied_for_op} =
diff --git a/lib/DBIx/Class/_Util.pm b/lib/DBIx/Class/_Util.pm
index ad438e7..0ab7ac9 100644
--- a/lib/DBIx/Class/_Util.pm
+++ b/lib/DBIx/Class/_Util.pm
@@ -177,7 +177,9 @@ sub is_plain_value ($) {
       # intersted in are much more limited than the fullblown thing, and
       # this is a relatively hot piece of code
       (
-        # either has stringification which DBI prefers out of the box
+        # FIXME - DBI needs fixing to stringify regardless of DBD
+        #
+        # either has stringification which DBI SHOULD prefer out of the box
         #first { *{$_ . '::(""'}{CODE} } @{ mro::get_linear_isa( ref $_[0] ) }
         overload::Method($_[0], '""')
           or
diff --git a/t/100populate.t b/t/100populate.t
index 27eb3ef..4a3f0ac 100644
--- a/t/100populate.t
+++ b/t/100populate.t
@@ -416,7 +416,7 @@ warnings_like {
   )
     ? ()
     # one unique for populate() and create() each
-    : (qr/\QPOSSIBLE *PAST* DATA CORRUPTION detected \E.+\QTrigger condition encountered at @{[ __FILE__ ]} line\E \d/) x 3
+    : (qr/\QPOSSIBLE *PAST* DATA CORRUPTION detected \E.+\QTrigger condition encountered at @{[ __FILE__ ]} line\E \d/) x 2
 ], 'Data integrity warnings as planned';
 
 lives_ok {