From: Peter Rabbitson <ribasushi@cpan.org>
Date: Tue, 5 Aug 2014 11:13:10 +0000 (+0200)
Subject: Deprecate insert_bulk - we will be changing its signature down the road
X-Git-Tag: v0.082800~84
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=2a6dda4b4b591e4da531d6c78ff9dc9e359d5fd9;p=dbsrgits%2FDBIx-Class.git

Deprecate insert_bulk - we will be changing its signature down the road

Besides there is really no reason for users to call it directly
---

diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm
index 2e2675e..97dfe78 100644
--- a/lib/DBIx/Class/ResultSet.pm
+++ b/lib/DBIx/Class/ResultSet.pm
@@ -2434,7 +2434,7 @@ sub populate {
 ### main source data
   # FIXME - need to switch entirely to a coderef-based thing,
   # so that large sets aren't copied several times... I think
-  $rsrc->storage->insert_bulk(
+  $rsrc->storage->_insert_bulk(
     $rsrc,
     [ @$colnames, sort keys %$rs_data ],
     [ map {
diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm
index fe503b4..483a6d0 100644
--- a/lib/DBIx/Class/Storage/DBI.pm
+++ b/lib/DBIx/Class/Storage/DBI.pm
@@ -101,12 +101,13 @@ for my $meth (keys %$storage_accessor_idx, qw(
   txn_begin
 
   insert
-  insert_bulk
   update
   delete
   select
   select_single
 
+  _insert_bulk
+
   with_deferred_fk_checks
 
   get_use_dbms_capability
@@ -2036,8 +2037,24 @@ sub insert {
 }
 
 sub insert_bulk {
+  carp_unique(
+    'insert_bulk() should have never been exposed as a public method and '
+  . 'calling it is depecated as of Aug 2014. If you believe having a genuine '
+  . 'use for this method please contact the development team via '
+  . DBIx::Class::_ENV_::HELP_URL
+  );
+
+  return '0E0' unless @{$_[3]||[]};
+
+  shift->_insert_bulk(@_);
+}
+
+sub _insert_bulk {
   my ($self, $source, $cols, $data) = @_;
 
+  $self->throw_exception('Calling _insert_bulk without a dataset to process makes no sense')
+    unless @{$data||[]};
+
   my $colinfos = $source->columns_info($cols);
 
   local $self->{_autoinc_supplied_for_op} =
@@ -2117,7 +2134,7 @@ sub insert_bulk {
     # if the bindlist is empty and we had some dynamic binds, this means the
     # storage ate them away (e.g. the NoBindVars component) and interpolated
     # them directly into the SQL. This obviously can't be good for multi-inserts
-    $self->throw_exception('Cannot insert_bulk without support for placeholders');
+    $self->throw_exception('Unable to invoke fast-path insert without storage placeholder support');
   }
 
   # sanity checks
diff --git a/lib/DBIx/Class/Storage/DBI/ADO/Microsoft_SQL_Server.pm b/lib/DBIx/Class/Storage/DBI/ADO/Microsoft_SQL_Server.pm
index 09cbee6..9bb93f9 100644
--- a/lib/DBIx/Class/Storage/DBI/ADO/Microsoft_SQL_Server.pm
+++ b/lib/DBIx/Class/Storage/DBI/ADO/Microsoft_SQL_Server.pm
@@ -189,9 +189,9 @@ sub _dbi_attrs_for_bind {
   return $attrs;
 }
 
-# Can't edit all the binds in _dbi_attrs_for_bind for insert_bulk, so we take
+# Can't edit all the binds in _dbi_attrs_for_bind for _insert_bulk, so we take
 # care of those GUIDs here.
-sub insert_bulk {
+sub _insert_bulk {
   my $self = shift;
   my ($source, $cols, $data) = @_;
 
diff --git a/lib/DBIx/Class/Storage/DBI/Replicated.pm b/lib/DBIx/Class/Storage/DBI/Replicated.pm
index fc10c75..0a73c4b 100644
--- a/lib/DBIx/Class/Storage/DBI/Replicated.pm
+++ b/lib/DBIx/Class/Storage/DBI/Replicated.pm
@@ -265,7 +265,6 @@ my $method_dispatch = {
     build_datetime_parser
     last_insert_id
     insert
-    insert_bulk
     update
     delete
     dbh
@@ -315,6 +314,8 @@ my $method_dispatch = {
     _native_data_type
     _get_dbh
     sql_maker_class
+    insert_bulk
+    _insert_bulk
     _execute
     _do_query
     _dbh_execute
diff --git a/lib/DBIx/Class/Storage/DBI/Sybase/ASE.pm b/lib/DBIx/Class/Storage/DBI/Sybase/ASE.pm
index 50a8f6b..252a50c 100644
--- a/lib/DBIx/Class/Storage/DBI/Sybase/ASE.pm
+++ b/lib/DBIx/Class/Storage/DBI/Sybase/ASE.pm
@@ -179,7 +179,7 @@ sub disconnect {
 
 # Even though we call $sth->finish for uses off the bulk API, there's still an
 # "active statement" warning on disconnect, which we throw away here.
-# This is due to the bug described in insert_bulk.
+# This is due to the bug described in _insert_bulk.
 # Currently a noop because 'prepare' is used instead of 'prepare_cached'.
   local $SIG{__WARN__} = sigwarn_silencer(qr/active statement/i)
     if $self->_is_bulk_storage;
@@ -501,7 +501,7 @@ sub update {
   }
 }
 
-sub insert_bulk {
+sub _insert_bulk {
   my $self = shift;
   my ($source, $cols, $data) = @_;
 
@@ -607,7 +607,7 @@ sub insert_bulk {
 # This ignores any data conversion errors detected by the client side libs, as
 # they are usually harmless.
   my $orig_cslib_cb = DBD::Sybase::set_cslib_cb(
-    Sub::Name::subname insert_bulk => sub {
+    Sub::Name::subname _insert_bulk_cslib_errhandler => sub {
       my ($layer, $origin, $severity, $errno, $errmsg, $osmsg, $blkmsg) = @_;
 
       return 1 if $errno == 36;
@@ -685,7 +685,7 @@ sub insert_bulk {
 
     $self->_bulk_storage(undef);
     unshift @_, $self;
-    goto \&insert_bulk;
+    goto \&_insert_bulk;
   }
   elsif ($exception) {
 # rollback makes the bulkLogin connection unusable
@@ -717,7 +717,7 @@ sub _remove_blob_cols {
   return %blob_cols ? \%blob_cols : undef;
 }
 
-# same for insert_bulk
+# same for _insert_bulk
 sub _remove_blob_cols_array {
   my ($self, $source, $cols, $data) = @_;
 
diff --git a/t/100populate.t b/t/100populate.t
index 57efc72..e7cb830 100644
--- a/t/100populate.t
+++ b/t/100populate.t
@@ -98,7 +98,7 @@ is(scalar @links, 2);
 is($links[0]->url, undef);
 is($links[1]->url, 'url42');
 
-## make sure populate -> insert_bulk honors fields/orders in void context
+## make sure populate -> _insert_bulk honors fields/orders in void context
 ## schema order
 $schema->populate('Link', [
 [ qw/id url title/ ],
@@ -191,7 +191,7 @@ is($links[2]->title, undef);
   my $rs = $schema->resultset('Link');
   $rs->delete;
 
-  # test insert_bulk with all literal sql (no binds)
+  # test populate with all literal sql (no binds)
 
   $rs->populate([
     (+{
@@ -229,7 +229,7 @@ is($links[2]->title, undef);
   my $rs = $schema->resultset('Link');
   $rs->delete;
 
-  # test insert_bulk with all literal/bind sql
+  # test populate with all literal/bind sql
   $rs->populate([
     (+{
         url => \['?', [ {} => 'cpan.org' ] ],
@@ -244,7 +244,7 @@ is($links[2]->title, undef);
 
   $rs->delete;
 
-  # test insert_bulk with mix literal and literal/bind
+  # test populate with mix literal and literal/bind
   $rs->populate([
     (+{
         url => \"'cpan.org'",
diff --git a/t/746sybase.t b/t/746sybase.t
index cb6849a..af1f7a3 100644
--- a/t/746sybase.t
+++ b/t/746sybase.t
@@ -207,9 +207,9 @@ SQL
     name => { -like => 'bulk artist %' }
   });
 
-# test insert_bulk using populate.
+# test _insert_bulk using populate.
   SKIP: {
-    skip 'insert_bulk not supported', 4
+    skip '_insert_bulk not supported', 4
       unless $storage_type !~ /NoBindVars/i;
 
     lives_ok {
@@ -227,25 +227,25 @@ SQL
           charfield => 'foo',
         },
       ]);
-    } 'insert_bulk via populate';
+    } '_insert_bulk via populate';
 
-    is $bulk_rs->count, 3, 'correct number inserted via insert_bulk';
+    is $bulk_rs->count, 3, 'correct number inserted via _insert_bulk';
 
     is ((grep $_->charfield eq 'foo', $bulk_rs->all), 3,
-      'column set correctly via insert_bulk');
+      'column set correctly via _insert_bulk');
 
     my %bulk_ids;
     @bulk_ids{map $_->artistid, $bulk_rs->all} = ();
 
     is ((scalar keys %bulk_ids), 3,
-      'identities generated correctly in insert_bulk');
+      'identities generated correctly in _insert_bulk');
 
     $bulk_rs->delete;
   }
 
-# make sure insert_bulk works a second time on the same connection
+# make sure _insert_bulk works a second time on the same connection
   SKIP: {
-    skip 'insert_bulk not supported', 3
+    skip '_insert_bulk not supported', 3
       unless $storage_type !~ /NoBindVars/i;
 
     lives_ok {
@@ -263,20 +263,20 @@ SQL
           charfield => 'bar',
         },
       ]);
-    } 'insert_bulk via populate called a second time';
+    } '_insert_bulk via populate called a second time';
 
     is $bulk_rs->count, 3,
-      'correct number inserted via insert_bulk';
+      'correct number inserted via _insert_bulk';
 
     is ((grep $_->charfield eq 'bar', $bulk_rs->all), 3,
-      'column set correctly via insert_bulk');
+      'column set correctly via _insert_bulk');
 
     $bulk_rs->delete;
   }
 
-# test invalid insert_bulk (missing required column)
+# test invalid _insert_bulk (missing required column)
 #
-# There should be a rollback, reconnect and the next valid insert_bulk should
+# There should be a rollback, reconnect and the next valid _insert_bulk should
 # succeed.
   throws_ok {
     $schema->resultset('Artist')->populate([
@@ -288,11 +288,11 @@ SQL
 # The second pattern is the error from fallback to regular array insert on
 # incompatible charset.
 # The third is for ::NoBindVars with no syb_has_blk.
-  'insert_bulk with missing required column throws error';
+  '_insert_bulk with missing required column throws error';
 
-# now test insert_bulk with IDENTITY_INSERT
+# now test _insert_bulk with IDENTITY_INSERT
   SKIP: {
-    skip 'insert_bulk not supported', 3
+    skip '_insert_bulk not supported', 3
       unless $storage_type !~ /NoBindVars/i;
 
     lives_ok {
@@ -313,13 +313,13 @@ SQL
           charfield => 'foo',
         },
       ]);
-    } 'insert_bulk with IDENTITY_INSERT via populate';
+    } '_insert_bulk with IDENTITY_INSERT via populate';
 
     is $bulk_rs->count, 3,
-      'correct number inserted via insert_bulk with IDENTITY_INSERT';
+      'correct number inserted via _insert_bulk with IDENTITY_INSERT';
 
     is ((grep $_->charfield eq 'foo', $bulk_rs->all), 3,
-      'column set correctly via insert_bulk with IDENTITY_INSERT');
+      'column set correctly via _insert_bulk with IDENTITY_INSERT');
 
     $bulk_rs->delete;
   }
@@ -434,7 +434,7 @@ SQL
 
     $rs->delete;
 
-    # now try insert_bulk with blobs and only blobs
+    # now try _insert_bulk with blobs and only blobs
     $new_str = $binstr{large} . 'bar';
     lives_ok {
       $rs->populate([
@@ -447,18 +447,18 @@ SQL
           clob => $new_str,
         },
       ]);
-    } 'insert_bulk with blobs does not die';
+    } '_insert_bulk with blobs does not die';
 
     is((grep $_->blob eq $binstr{large}, $rs->all), 2,
-      'IMAGE column set correctly via insert_bulk');
+      'IMAGE column set correctly via _insert_bulk');
 
     is((grep $_->clob eq $new_str, $rs->all), 2,
-      'TEXT column set correctly via insert_bulk');
+      'TEXT column set correctly via _insert_bulk');
 
-    # now try insert_bulk with blobs and a non-blob which also happens to be an
+    # now try _insert_bulk with blobs and a non-blob which also happens to be an
     # identity column
     SKIP: {
-      skip 'no insert_bulk without placeholders', 4
+      skip 'no _insert_bulk without placeholders', 4
         if $storage_type =~ /NoBindVars/i;
 
       $rs->delete;
@@ -480,16 +480,16 @@ SQL
             a_memo => 2,
           },
         ]);
-      } 'insert_bulk with blobs and explicit identity does NOT die';
+      } '_insert_bulk with blobs and explicit identity does NOT die';
 
       is((grep $_->blob eq $binstr{large}, $rs->all), 2,
-        'IMAGE column set correctly via insert_bulk with identity');
+        'IMAGE column set correctly via _insert_bulk with identity');
 
       is((grep $_->clob eq $new_str, $rs->all), 2,
-        'TEXT column set correctly via insert_bulk with identity');
+        'TEXT column set correctly via _insert_bulk with identity');
 
       is_deeply [ map $_->id, $rs->all ], [ 1,2 ],
-        'explicit identities set correctly via insert_bulk with blobs';
+        'explicit identities set correctly via _insert_bulk with blobs';
     }
 
     lives_and {