Fix spurious ROLLBACK statements when a TxnScopeGuard fails a deferred commit

( cherry-pick of 0bec44d5d7 )

This is a *temporary workaround*, and I am really unhappy about it... it just
feels wrong. See the comment in lib/DBIx/Class/Storage/TxnScopeGuard.pm

diff --git a/Changes b/Changes
index b6c3296..062797e 100644 (file)
--- a/Changes
+++ b/Changes
@@ -8,6 +8,9 @@ Revision history for DBIx::Class
         - Fix use of ::Schema::Versioned combined with a user-supplied
           $dbh->{HandleError} (GH#101)
         - Fix parsing of DSNs containing driver arguments (GH#99)
+        - Fix spurious ROLLBACK statements when a TxnScopeGuard fails a commit
+          of a transaction with deferred FK checks: a guard is now inactivated
+          immediately before the commit is attempted (RT#107159)
         - Fix spurious warning on MSSQL cursor invalidation retries (RT#102166)
         - Remove spurious exception warping in ::Replicated::execute_reliably
           (RT#113339)

diff --git a/lib/DBIx/Class/Storage/TxnScopeGuard.pm b/lib/DBIx/Class/Storage/TxnScopeGuard.pm
index edf7205..392e354 100644 (file)
--- a/lib/DBIx/Class/Storage/TxnScopeGuard.pm
+++ b/lib/DBIx/Class/Storage/TxnScopeGuard.pm
@@ -45,8 +45,13 @@ sub commit {
   $self->{storage}->throw_exception("Refusing to execute multiple commits on scope guard $self")
     if $self->{inactivated};
 
-  $self->{storage}->txn_commit;
+  # FIXME - this assumption may be premature: a commit may fail and a rollback
+  # *still* be necessary. Currently I am not aware of such scenarious, but I
+  # also know the deferred constraint handling is *severely* undertested.
+  # Making the change of "fire txn and never come back to this" in order to
+  # address RT#107159, but this *MUST* be reevaluated later.
   $self->{inactivated} = 1;
+  $self->{storage}->txn_commit;
 }
 
 sub DESTROY {

diff --git a/t/72pg.t b/t/72pg.t
index 32e0cc1..c1105fd 100644 (file)
--- a/t/72pg.t
+++ b/t/72pg.t
@@ -3,6 +3,7 @@ use warnings;
 
 use Test::More;
 use Test::Exception;
+use Test::Warn;
 use Sub::Name;
 use Config;
 use DBIx::Class::Optional::Dependencies ();
@@ -460,7 +461,28 @@ lives_ok { $cds->update({ year => '2010' }) } 'Update on prefetched rs';
     $schema->resultset('Track')->create({
       trackid => 1, cd => 9999, position => 1, title => 'Track1'
     });
-  } qr/constraint/i, 'with_deferred_fk_checks is off';
+  } qr/violates foreign key constraint/i, 'with_deferred_fk_checks is off outside of TXN';
+
+  # rerun the same under with_deferred_fk_checks
+  # it is expected to fail, hence the eval
+  # but it also should not warn
+  warnings_like {
+
+    # workaround for PG 9.5, fix pending in mainline
+    local $schema->storage->_dbh->{PrintWarn} = 0;
+
+    eval {
+      $schema->storage->with_deferred_fk_checks(sub {
+        $schema->resultset('Track')->create({
+          trackid => 1, cd => 9999, position => 1, title => 'Track1'
+        });
+      } )
+    };
+
+    like $@, qr/violates foreign key constraint/i,
+      "Still expected exception on deferred failure at commit time";
+
+  } [], 'No warnings on deferred rollback';
 }
 
 done_testing;