Skip Menu |

This queue is for tickets about the DBIx-Class CPAN distribution.

Report information
The Basics
Id: 129816
Status: open
Priority: 0/
Queue: DBIx-Class
People
Owner: Nobody in particular
Requestors: ether [...] cpan.org
Cc:
AdminCc:
Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)

History Show all quoted text
  Thu Jun 13 13:45:50 2019 ether [...] cpan.org - Ticket created
Subject: BlockRunner not handling rollback-during-commit gracefully
(#dbix-class, 2019-06-13) Hopefully this is enough information. Because the issue involves interacting with a real db driver, I don't think it's possible to easily write this up as a unit test. 10:22 < ether> bah, it looks like deferred constraint checking is not well-handled in DBD::Pg... 10:22 < ether> I've got a bunch of statements wrapped in a transaction. some of the constraints deferred until that 'COMMIT' is executed... 10:23 < ether> I'm using txn_do, so a ROLLBACK is then triggered 10:23 < ether> and then I get a warning: rollback ineffective with AutoCommit enabled at /Users/ether/.perlbrew/libs/26.3@std/lib/perl5/DBIx/Class/Storage/DBI.pm line 1651. 10:24 < ether> which is a red herring in this case, because everything did actually get successfully rolled back 10:24 < ether> I can't find this warning string in .pm code so it must be in XS somewhere, in the driver itself 10:25 < ether> all I can think to do is to set up a temporary __WARN__ handler to swallow this warning, where I set up the txn_do 10:26 < ether> ah, it's in Driver.xst in the DBI distribution 10:27 < ether> https://metacpan.org/source/TIMB/DBI-1.642/Driver.xst#L305-312 10:28 < ether> I'm not quite sure how one could patch this for this situation, though 10:29 < ether> perhaps I could temporarily turn off AutoCommit on the dbh 10:29 < ether> or should DBIx::Class::Storage::txn_rollback be doing this? 10:30 < alh> Why is autocommit on if you're in a tx? 10:31 < ether> I don't think it get turned off for transactions 10:34 < mohawk> i thought it did 10:34 < ether> I don't have this problem for normal rollbacks inside transactions -- this one is special because we already issued a 'COMMIT' and the exception (triggering a rollback) happened during that execution 10:34 < ether> (because of the deferred constraints) 10:35 < mohawk> sounds like DBI itself might be turning autocommit back on a bit prematurely to me 10:35 <@mst> oh, that's interesting 10:35 <@mst> mohawk: naw 10:36 < ether> ? 10:36 <@mst> this'll be "DBIC tries to rollback on exception, it hadn't occurred to DBIC that commit could be what's throwing it" 10:36 <@mst> I'd bet 10:36 <@mst> which means it's not really DBD::Pg's fault 10:37 < ether> if it's DBIx::Class's fault, that means I should be able to patch around it 10:37 < ether> adding a __WARN__ handler is hacky but perhaps safest 10:38 <@mst> BlockRunner.pm seems to be where it's getting confused 10:40 < mohawk> mst, does that mean DBIC itself is changing AutoCommit? 10:40 <@mst> what 10:41 <@mst> of course it is, that's how transactions *work* 10:41 < ether> I thought the 'start transaction' statement might override AutoCommit 10:41 < mohawk> the last part i am aware of. my mental model was that DBI (not DBIC) took care of all of that, and DBIC just called txn_do (which i will now confirm is a DBI thing by looking that up) 10:42 <@mst> no, txn_do I invented 10:42 <@mst> however, calling $dbh->begin_work will IIRC set AutoCommit to 0 10:43 < mohawk> mst, thanks - that was where my model was wrong 10:44 < hobbs> txn_do appeared in DBIC and then was extracted into a couple modules which can provide the same thing on top of DBI without the rest of DBIC 10:44 <@mst> right 10:44 <@mst> because once I'd implemented it, it was obviously superior, so in the true spirit of open source people stole it enough times that it just started to seem obvious to people
  Sat Jun 15 05:22:48 2019 ribasushi [...] leporine.io - Correspondence added
Subject: Re: [rt.cpan.org #129816] BlockRunner not handling rollback-during-commit gracefully
Date: Sat, 15 Jun 2019 11:17:15 +0200
To: bug-DBIx-Class [...] rt.cpan.org
From: Peter Rabbitson <ribasushi [...] leporine.io>
On 06/13/2019 07:45 PM, Karen Etheridge via RT wrote: Show quoted text
> > Hopefully this is enough information.
I am unable to glean much insight from the attached log. Show quoted text
> Because the issue involves interacting with a real db driver, I don't think it's possible to easily write this up as a unit test.
This assumption is incorrect. Please augment the currently passing test to mimic your failure-case: https://github.com/Perl5/DBIx-Class/blob/maint/0.0828xx/t/72pg.t#L442-L490 A fix would be rather obvious once your failure scenario is understood.
  Sat Jun 15 05:22:49 2019 The RT System itself - Status changed from 'new' to 'open'
  Tue Jul 23 04:27:17 2019 ribasushi [...] leporine.io - Correspondence added
On Sat Jun 15 11:22:48 2019, RIBASUSHI wrote: Show quoted text
> Please augment the currently passing > test > to mimic your failure-case: > https://github.com/Perl5/DBIx-Class/blob/maint/0.0828xx/t/72pg.t#L442- > L490 > > A fix would be rather obvious once your failure scenario is > understood.
Ping?
  Tue Jul 23 17:42:50 2019 ether [...] cpan.org - Correspondence added
On 2019-07-23 01:27:17, RIBASUSHI wrote: Show quoted text
> On Sat Jun 15 11:22:48 2019, RIBASUSHI wrote: >
> > Please augment the currently passing > > test > > to mimic your failure-case: > > https://github.com/Perl5/DBIx-Class/blob/maint/0.0828xx/t/72pg.t#L442- > > L490 > > > > A fix would be rather obvious once your failure scenario is > > understood.
> > > Ping?
See attached patch. With DBIC_TRACE=1, it creates the following output: BEGIN WORK # starting transaction INSERT INTO artist ( name, rank) VALUES ( ?, ? ) RETURNING artistid: 'bob', '1' INSERT INTO artist ( name, rank) VALUES ( ?, ? ) RETURNING artistid: 'bob', '2' COMMIT ROLLBACK rollback ineffective with AutoCommit enabled at lib/DBIx/Class/Storage/DBI.pm line 1651. not ok 511 - got no exception on rollback # Failed test 'got no exception on rollback' # at t/72pg.t line 518.
Subject: 0001-repro-case-for-RT-129816.patch
From d19577cf257764d507bf778847b9728f34ee5308 Mon Sep 17 00:00:00 2001 From: Karen Etheridge <ether@cpan.org> Date: Tue, 23 Jul 2019 14:40:47 -0700 Subject: [PATCH] repro case for RT#129816 --- t/72pg.t | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/t/72pg.t b/t/72pg.t index e9c5ce16..d7fa2e5c 100644 --- a/t/72pg.t +++ b/t/72pg.t @@ -489,6 +489,35 @@ lives_ok { $cds->update({ year => '2010' }) } 'Update on prefetched rs'; } [], 'No warnings on deferred rollback'; } +# repro case for https://rt.cpan.org/Ticket/Display.html?id=129816 +{ + my $schema = DBICTest::Schema->connect($dsn, $user, $pass); + drop_test_schema($schema); + create_test_schema($schema); + use Try::Tiny; + my $exception; + try { + $schema->txn_do(sub { + print STDERR "# starting transaction\n"; + $schema->storage->dbh_do(sub { $_[1]->do('set constraints all deferred') }); + + # now do something that will cause a constraint violation *at commit time* + $schema->resultset('Artist')->create({ + name => 'bob', rank => 1, + }); + $schema->resultset('Artist')->create({ + name => 'bob', rank => 2, + }); + + }); + } + catch { + $exception = $_; + }; + + ok(!$exception, 'got no exception on rollback'); +} + done_testing; END { @@ -511,7 +540,7 @@ sub create_test_schema { my $std_artist_table = <<EOS; ( artistid serial PRIMARY KEY - , name VARCHAR(100) + , name VARCHAR(100) unique deferrable , rank INTEGER NOT NULL DEFAULT '13' , charfield CHAR(10) , arrayfield INTEGER[] -- 2.22.0
  Tue Jul 23 17:45:25 2019 ether [...] cpan.org - Correspondence added
Sorry, the test on $exception there is wrong -- of course we expect an exception, we just don't expect a warning. Nevertheless, it reproduces the issue I was describing -- a warning is generated on commit->die->rollback that should be suppressed.
  Tue Jul 23 17:51:32 2019 ether [...] cpan.org - Correspondence added
Better patch. Output: ... BEGIN WORK INSERT INTO artist ( name, rank) VALUES ( ?, ? ) RETURNING artistid: 'bob', '1' INSERT INTO artist ( name, rank) VALUES ( ?, ? ) RETURNING artistid: 'bob', '2' COMMIT ROLLBACK not ok 511 - got no warnings on rollback # Failed test 'got no warnings on rollback' # at t/72pg.t line 513. # got warnings: rollback ineffective with AutoCommit enabled at lib/DBIx/Class/Storage/DBI.pm line 1651.
Subject: 0001-repro-case-for-RT-129816.patch
From ac27d3e4c1950316fbb1c061a9589d5c5f193160 Mon Sep 17 00:00:00 2001 From: Karen Etheridge <ether@cpan.org> Date: Tue, 23 Jul 2019 14:40:47 -0700 Subject: [PATCH] repro case for RT#129816 --- t/72pg.t | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/t/72pg.t b/t/72pg.t index e9c5ce16..a1570a59 100644 --- a/t/72pg.t +++ b/t/72pg.t @@ -489,6 +489,31 @@ lives_ok { $cds->update({ year => '2010' }) } 'Update on prefetched rs'; } [], 'No warnings on deferred rollback'; } +# repro case for https://rt.cpan.org/Ticket/Display.html?id=129816 +{ + my $schema = DBICTest::Schema->connect($dsn, $user, $pass); + drop_test_schema($schema); + create_test_schema($schema); + my @warnings; + local $SIG{__WARN__} = sub { push @warnings, $_[0] }; + eval { + $schema->txn_do(sub { + $schema->storage->dbh_do(sub { $_[1]->do('set constraints all deferred') }); + + # now do something that will cause a constraint violation *at commit time* + $schema->resultset('Artist')->create({ + name => 'bob', rank => 1, + }); + $schema->resultset('Artist')->create({ + name => 'bob', rank => 2, + }); + }); + }; + + ok(!@warnings, 'got no warnings on rollback') + or diag 'got warnings: ', explain @warnings; +} + done_testing; END { @@ -511,7 +536,7 @@ sub create_test_schema { my $std_artist_table = <<EOS; ( artistid serial PRIMARY KEY - , name VARCHAR(100) + , name VARCHAR(100) unique deferrable , rank INTEGER NOT NULL DEFAULT '13' , charfield CHAR(10) , arrayfield INTEGER[] -- 2.22.0