From f7dde96931042ebf1eebba4cb1e43ebd94782818 Mon Sep 17 00:00:00 2001 From: Kenichi Ishigaki Date: Tue, 4 Aug 2015 19:12:58 +0900 Subject: [PATCH] outermost savepoint should be treated as a begin(RT #106151) --- MANIFEST | 1 + dbdimp.c | 126 +++++++----------------------- t/rt_106151_outermost_savepoint.t | 87 +++++++++++++++++++++ 3 files changed, 116 insertions(+), 98 deletions(-) create mode 100644 t/rt_106151_outermost_savepoint.t diff --git a/MANIFEST b/MANIFEST index e3eacde..2634c2f 100644 --- a/MANIFEST +++ b/MANIFEST @@ -83,6 +83,7 @@ t/58_see_if_its_a_number_and_explicit_binding.t t/59_extended_result_codes.t t/cookbook_variance.t t/lib/Test.pm +t/rt_106151_outermost_savepoint.t t/rt_15186_prepcached.t t/rt_21406_auto_finish.t t/rt_25371_asymmetric_unicode.t diff --git a/dbdimp.c b/dbdimp.c index d0c0da1..d7515ed 100644 --- a/dbdimp.c +++ b/dbdimp.c @@ -52,82 +52,25 @@ DBISTATE_DECLARE; bool _starts_with_begin(const char *sql) { return ( - (sql[0] == 'B' || sql[0] == 'b') && - (sql[1] == 'E' || sql[1] == 'e') && - (sql[2] == 'G' || sql[2] == 'g') && - (sql[3] == 'I' || sql[3] == 'i') && - (sql[4] == 'N' || sql[4] == 'n') + ((sql[0] == 'B' || sql[0] == 'b') && + (sql[1] == 'E' || sql[1] == 'e') && + (sql[2] == 'G' || sql[2] == 'g') && + (sql[3] == 'I' || sql[3] == 'i') && + (sql[4] == 'N' || sql[4] == 'n') + ) || ( + (sql[0] == 'S' || sql[0] == 's') && + (sql[1] == 'A' || sql[1] == 'a') && + (sql[2] == 'V' || sql[2] == 'v') && + (sql[3] == 'E' || sql[3] == 'e') && + (sql[4] == 'P' || sql[4] == 'p') && + (sql[5] == 'O' || sql[5] == 'o') && + (sql[6] == 'I' || sql[6] == 'i') && + (sql[7] == 'N' || sql[7] == 'n') && + (sql[8] == 'T' || sql[8] == 't') + ) ) ? TRUE : FALSE; } -bool -_starts_with_commit(const char *sql) { - return ( - ((sql[0] == 'C' || sql[0] == 'c') && - (sql[1] == 'O' || sql[1] == 'o') && - (sql[2] == 'M' || sql[2] == 'm') && - (sql[3] == 'M' || sql[3] == 'm') && - (sql[4] == 'I' || sql[4] == 'i') && - (sql[5] == 'T' || sql[5] == 't')) || - ((sql[0] == 'E' || sql[0] == 'e') && - (sql[1] == 'N' || sql[1] == 'n') && - (sql[2] == 'D' || sql[2] == 'd')) - ) ? TRUE : FALSE; -} - -bool -_starts_with_rollback(const char *sql) { - int i; - if ( - (sql[0] == 'R' || sql[0] == 'r') && - (sql[1] == 'O' || sql[1] == 'o') && - (sql[2] == 'L' || sql[2] == 'l') && - (sql[3] == 'L' || sql[3] == 'l') && - (sql[4] == 'B' || sql[4] == 'b') && - (sql[5] == 'A' || sql[5] == 'a') && - (sql[6] == 'C' || sql[6] == 'c') && - (sql[7] == 'K' || sql[7] == 'k')) { - int l = strlen(sql); - bool is_savepoint = FALSE; - for(i = 8; i < l; i++) { - if (_isspace(sql[i])) continue; - if (sql[i] == '-' && sql[i+1] == '-') { - while (sql[i] != 0 && sql[i] != '\n') i++; - continue; - } - if (sql[i] == 'T' || sql[i] == 't') { - if ( - (sql[i+0] == 'T' || sql[i+0] == 't') && - (sql[i+1] == 'R' || sql[i+1] == 'r') && - (sql[i+2] == 'A' || sql[i+2] == 'a') && - (sql[i+3] == 'N' || sql[i+3] == 'n') && - (sql[i+4] == 'S' || sql[i+4] == 's') && - (sql[i+5] == 'A' || sql[i+5] == 'a') && - (sql[i+6] == 'C' || sql[i+6] == 'c') && - (sql[i+7] == 'T' || sql[i+7] == 't') && - (sql[i+8] == 'I' || sql[i+8] == 'i') && - (sql[i+9] == 'O' || sql[i+9] == 'o') && - (sql[i+10] == 'N' || sql[i+10] == 'n')) { - i += 10; continue; - } - else if ( - (sql[i+0] == 'T' || sql[i+0] == 't') && - (sql[i+1] == 'O' || sql[i+1] == 'o') && - (sql[i+2] == ' ' || sql[i+2] == '\t')) { - /* rolling back to a savepoint should not - change AutoCommit status */ - is_savepoint = TRUE; - } - } - break; - } - if (!is_savepoint) { - return TRUE; - } - } - return FALSE; -} - /* adopted from sqlite3.c */ #define LARGEST_INT64 (0xffffffff|(((sqlite3_int64)0x7fffffff)<<32)) @@ -567,18 +510,6 @@ sqlite_db_do_sv(SV *dbh, imp_dbh_t *imp_dbh, SV *sv_statement) } } } - else if (DBIc_is(imp_dbh, DBIcf_BegunWork)) { - const char *sql = statement; - _skip_whitespaces(sql); - if (_starts_with_commit(sql)) { - DBIc_off(imp_dbh, DBIcf_BegunWork); - DBIc_on(imp_dbh, DBIcf_AutoCommit); - } - else if (_starts_with_rollback(sql)) { - DBIc_off(imp_dbh, DBIcf_BegunWork); - DBIc_on(imp_dbh, DBIcf_AutoCommit); - } - } rc = sqlite_exec(dbh, statement); if (rc != SQLITE_OK) { @@ -586,6 +517,11 @@ sqlite_db_do_sv(SV *dbh, imp_dbh_t *imp_dbh, SV *sv_statement) return -2; } + if (DBIc_is(imp_dbh, DBIcf_BegunWork) && sqlite3_get_autocommit(imp_dbh->db)) { + DBIc_off(imp_dbh, DBIcf_BegunWork); + DBIc_on(imp_dbh, DBIcf_AutoCommit); + } + return sqlite3_changes(imp_dbh->db); } @@ -1051,19 +987,6 @@ sqlite_st_execute(SV *sth, imp_sth_t *imp_sth) } } } - else if (DBIc_is(imp_dbh, DBIcf_BegunWork)) { - /* COMPAT: sqlite3_sql is only available for 3006000 or newer */ - const char *sql = sqlite3_sql(imp_sth->stmt); - _skip_whitespaces(sql); - if (_starts_with_commit(sql)) { - DBIc_off(imp_dbh, DBIcf_BegunWork); - DBIc_on(imp_dbh, DBIcf_AutoCommit); - } - else if (_starts_with_rollback(sql)) { - DBIc_off(imp_dbh, DBIcf_BegunWork); - DBIc_on(imp_dbh, DBIcf_AutoCommit); - } - } imp_sth->nrow = 0; @@ -1079,6 +1002,13 @@ sqlite_st_execute(SV *sth, imp_sth_t *imp_sth) } return -5; /* -> undef in SQLite.xsi */ } + + /* transaction ended with commit/rollback/release */ + if (DBIc_is(imp_dbh, DBIcf_BegunWork) && sqlite3_get_autocommit(imp_dbh->db)) { + DBIc_off(imp_dbh, DBIcf_BegunWork); + DBIc_on(imp_dbh, DBIcf_AutoCommit); + } + /* warn("Finalize\n"); */ sqlite3_reset(imp_sth->stmt); imp_sth->nrow = sqlite3_changes(imp_dbh->db); diff --git a/t/rt_106151_outermost_savepoint.t b/t/rt_106151_outermost_savepoint.t new file mode 100644 index 0000000..6dc3eeb --- /dev/null +++ b/t/rt_106151_outermost_savepoint.t @@ -0,0 +1,87 @@ +#!/usr/bin/perl + +use strict; +BEGIN { + $| = 1; + $^W = 1; +} + +use t::lib::Test; +use Test::More; + +BEGIN { requires_sqlite('3.6.8') } + +plan tests => 7; +use Test::NoWarnings; + +{ # simple case + my $dbh = connect_ok( + AutoCommit => 1, + RaiseError => 1, + ); + $dbh->do("SAVEPOINT svp_0"); + $dbh->selectall_arrayref("SELECT * FROM sqlite_master"); + $dbh->do("RELEASE svp_0"); + # should not spit the "Issuing rollback()" warning +} + +{ # nested + my $dbh = connect_ok( + AutoCommit => 1, + RaiseError => 1, + ); + $dbh->do("SAVEPOINT svp_0"); + $dbh->do("SAVEPOINT svp_1"); + $dbh->selectall_arrayref("SELECT * FROM sqlite_master"); + $dbh->do("RELEASE svp_1"); + $dbh->do("RELEASE svp_0"); + # should not spit the "Issuing rollback()" warning +} + +{ # end with commit + my $dbh = connect_ok( + AutoCommit => 1, + RaiseError => 1, + ); + $dbh->do("SAVEPOINT svp_0"); + $dbh->do("SAVEPOINT svp_1"); + $dbh->selectall_arrayref("SELECT * FROM sqlite_master"); + $dbh->do("COMMIT"); + # should not spit the "Issuing rollback()" warning +} + +{ # end with rollback + my $dbh = connect_ok( + AutoCommit => 1, + RaiseError => 1, + ); + $dbh->do("SAVEPOINT svp_0"); + $dbh->do("SAVEPOINT svp_1"); + $dbh->selectall_arrayref("SELECT * FROM sqlite_master"); + $dbh->do("ROLLBACK"); + # should not spit the "Issuing rollback()" warning +} + +{ # end with outermost release + my $dbh = connect_ok( + AutoCommit => 1, + RaiseError => 1, + ); + $dbh->do("SAVEPOINT svp_0"); + $dbh->do("SAVEPOINT svp_1"); + $dbh->selectall_arrayref("SELECT * FROM sqlite_master"); + $dbh->do("RELEASE svp_0"); + # should not spit the "Issuing rollback()" warning +} + +{ # end by setting AutoCommit to true + my $dbh = connect_ok( + AutoCommit => 1, + RaiseError => 1, + ); + $dbh->do("SAVEPOINT svp_0"); + $dbh->do("SAVEPOINT svp_1"); + $dbh->selectall_arrayref("SELECT * FROM sqlite_master"); + $dbh->{AutoCommit} = 1; + # should not spit the "Issuing rollback()" warning +}