[PATCH 2/2] Add workaround to reset all statements on $dbh->rollback failure

[Yuriy M. Kaminskiy]

DBI don't expect and cannot correctly handle error in $dbh->rollback,
thus we are adding workaround to prevent this very surprising error
with nasty consequences.
$dbh->do("ROLLBACK") still produces error, but that's should be more
expected (and proper error handling in this case is not blocked by DBI).
---
dbdimp.c | 33 +++++++++++++++++++-
lib/DBD/SQLite.pm | 6 ++--
t/49_rollback_with_active_stmt.t | 62 ++++++++++++++++++++++++++++++++++++++
3 files changed, 97 insertions(+), 4 deletions(-)
create mode 100644 t/49_rollback_with_active_stmt.t

diff --git a/dbdimp.c b/dbdimp.c
index f77d93c..7d90f29 100644
--- a/dbdimp.c
+++ b/dbdimp.c
@@ -358,11 +358,42 @@ sqlite_db_rollback(SV *dbh, imp_dbh_t *imp_dbh)
croak_if_db_is_null();

if (!sqlite3_get_autocommit(imp_dbh->db)) {
+ char *errmsg = NULL;

sqlite_trace(dbh, imp_dbh, 3, "ROLLBACK TRAN");

- rc = sqlite_exec(dbh, "ROLLBACK TRANSACTION");
+ rc = sqlite3_exec(imp_dbh->db,
+ "ROLLBACK TRANSACTION",
+ NULL, NULL, &errmsg);
+
+ if (rc == SQLITE_BUSY && DBIc_ACTIVE_KIDS(imp_dbh)) {
+ sqlite3_stmt *pStmt = NULL;
+
+ warn("ROLLBACK failed: %s, trying to reset all statements and retry (there are %d active statement handle(s))", errmsg, DBIc_ACTIVE_KIDS(imp_dbh));
+ if (errmsg) {
+ sqlite3_free(errmsg);
+ errmsg = NULL;
+ }
+
+ /* COMPAT: sqlite3_next_stmt is only available for 3006000 or newer */
+ while ((pStmt = sqlite3_next_stmt(imp_dbh->db, pStmt)) != NULL) {
+ /* FIXME:
+ * This leaves $sth in somewhat confused state;
+ * It would be better to iterate through active DBI sth's and
+ * call sqlite_st_finish(), but I have no idea how.
+ */
+ sqlite3_reset(pStmt);
+ }
+ sqlite_trace(dbh, imp_dbh, 3, "ROLLBACK TRAN [retry]");
+ rc = sqlite3_exec(imp_dbh->db,
+ "ROLLBACK TRANSACTION",
+ NULL, NULL, &errmsg);
+ }
+
if (rc != SQLITE_OK) {
+ sqlite_error(dbh, rc, errmsg);
+ if (errmsg) sqlite3_free(errmsg);
+
return FALSE; /* -> &sv_no in SQLite.xsi */
}
}
diff --git a/lib/DBD/SQLite.pm b/lib/DBD/SQLite.pm
index 5abaaaa..76576f8 100644
--- a/lib/DBD/SQLite.pm
+++ b/lib/DBD/SQLite.pm
@@ -959,7 +959,7 @@ C<SELECT> statements is not optional with sqlite, otherwise
C<ROLLBACK> will fail.

$sth = $dbh->prepare("SELECT * FROM t");
- $dbh->begin_work;
+ $dbh->do("BEGIN");
eval {
$sth->execute;
$row = $sth->fetch;
@@ -968,9 +968,9 @@ C<ROLLBACK> will fail.
...
};
if($@) {
- $dbh->rollback; # UNEXPECTEDLY FAILS!
+ $dbh->do("ROLLBACK"); # UNEXPECTEDLY FAILS!
} else {
- $dbh->commit;
+ $dbh->do("COMMIT");
}

With database modification statements returning no data
diff --git a/t/49_rollback_with_active_stmt.t b/t/49_rollback_with_active_stmt.t
new file mode 100644
index 0000000..d010539
--- /dev/null
+++ b/t/49_rollback_with_active_stmt.t
@@ -0,0 +1,62 @@
+#!/usr/bin/perl
+
+# Trigger error on ROLLBACK, test workaround in $dbh->rollback
+
+use strict;
+BEGIN {
+ $| = 1;
+ $^W = 1;
+}
+
+use t::lib::Test qw/connect_ok dbfile/;
+use Test::More;
+use Test::NoWarnings;
+
+plan tests => 9 + 1;
+
+my $dbh = connect_ok(
+ dbfile => 'foo',
+ RaiseError => 1,
+ PrintError => 0,
+ AutoCommit => 1,
+);
+
+my $dbfile = dbfile('foo');
+
+$dbh->do("CREATE TABLE Blah ( id INTEGER )");
+$dbh->do("INSERT INTO Blah VALUES ( 1 )");
+$dbh->do("INSERT INTO Blah VALUES ( 2 )");
+$dbh->do("INSERT INTO Blah VALUES ( 3 )");
+
+my $sth;
+
+ok(($sth = $dbh->prepare("SELECT id FROM Blah")), "prepare");
+$dbh->do("BEGIN");
+$dbh->do("INSERT INTO Blah VALUES ( 4 )");
+
+$sth->execute;
+ok(ref($sth->fetch), "fetch");
+eval {
+ $dbh->do("ROLLBACK");
+};
+ok($@, "ROLLBACK fails with active SELECT statement");
+ok(ref($sth->fetch), "fetch 2nd row");
+if ($@) {
+ print "# expected execute failure : $@";
+}
+ok($sth->finish, "finish()");
+$dbh->do("ROLLBACK");
+$dbh->begin_work;
+$sth->execute;
+ok(ref($sth->fetch), "fetch");
+{
+ local $SIG{__WARN__} = sub {print "# expected warning : ",@_};
+ eval {$dbh->rollback};
+}
+ok(!$@, "rollback successful");
+ok($sth->finish, "finish()");
+$dbh->disconnect;
+
+undef($dbh);
+
+unlink $dbfile;