Skip Menu |

This queue is for tickets about the DBD-SQLite CPAN distribution.

Report information
The Basics
Id: 6340
Status: resolved
Priority: 0/
Queue: DBD-SQLite

People
Owner: Nobody in particular
Requestors: gm [...] virtuasites.com.br
Cc:
AdminCc:

Bug Information
Severity: Critical
Broken in: 0.27
Fixed in: (no value)



Subject: multiple INSERTIONs won't work if you have more than 1 STH opened/cached
I'm sending to you a script that shows a strange error when using insertions. The problems exits when you open a STH for insertion, create and execute a 2nd STH, than you execute the 1st STH. When I was using DBD::SQlite 0.25 everything worked fine, but from version 0.27 not! I tested with this versions: 0.25 - OK 0.26 - OK 0.27 - ERROR 0.29 - ERROR 0.31 - ERROR I'm on Win32 with Perl-5.6.1 (standart). I also have tested with DBI 1.37 and 1.42, and the DBI version is not influencing in the error. I tried to isolate the error in the script attached to help in the fix. Thanks to turn SQLITE available for the Perl community. Best regards, Graciliano M. P.
use DBI ; my $db = 'testerror.db' ; unlink($db) ; my $dbh = DBI->connect("dbi:SQLite:dbname=$db", '' , '' , { RaiseError => 0 , PrintError => 1 , AutoCommit => 1 }) ; $dbh->do('CREATE TABLE words (word TEXT , ids TEXT , id INTEGER PRIMARY KEY)') ; for (1..3) { print "______________________________________\n" ; print "LOOP $_\n" ; my $sth = $dbh->prepare('INSERT INTO words VALUES (?,?,?)') ; $sth->{ShowErrorStatement} = 1 ; my $sth2 ; $sth2 = $dbh->prepare( 'SELECT * FROM words LIMIT 1' ) ; $sth2->execute ; $sth->execute('q',2,undef) ; }
From: david_dick [...] iprimus.com.au
G'day Graciliano, Thank you for the excellent bug report. I can only hope you find this reply as useful. The feature that changed in the versions you mentioned is the method for retrieving data from the database. A full discussion on the old and new methods for retrieving data can be found at http://www.sqlite.org/c_interface.html. DBD::SQLite switched from using sqlite_exec, to using the replacement functions of sqlite_compile, sqlite_step and sqlite_finalize. Your problem was actually one statement locking the database and not releasing the lock in time. The second problem was that the error message that is returned is incorrect. The attached patch contains a quick patch that, when applied, will produce the following output from your sample program: Show quoted text
______________________________________ LOOP 1
______________________________________ LOOP 2 DBD::SQLite::st execute failed: database table is locked [for Statement "INSERT INTO words VALUES (?,?,?)"] at ./SQLITE-ERROR.pl line 27.
______________________________________ LOOP 3 DBD::SQLite::st execute failed: database table is locked [for Statement "INSERT INTO words VALUES (?,?,?)"] at ./SQLITE-ERROR.pl line 27. I hope this is helpful. Uru -Dave
diff -Naur DBD-SQLite-0.31/vdbe.c new/vdbe.c --- DBD-SQLite-0.31/vdbe.c Sun Feb 15 06:14:50 2004 +++ new/vdbe.c Sat May 22 20:07:42 2004 @@ -4809,6 +4809,7 @@ vdbe_halt: if( rc ){ p->rc = rc; + sqliteSetString(&p->zErrMsg, sqlite_error_string(rc), (char*)0); rc = SQLITE_ERROR; }else{ rc = SQLITE_DONE;
[guest - Sat May 22 06:23:25 2004]: Show quoted text
> G'day Graciliano, > Thank you for the excellent bug report. I can only hope you find this > reply as useful. The feature that changed in the versions you > mentioned > is the method for retrieving data from the database. A full > discussion > on the old and new methods for retrieving data can be found at > http://www.sqlite.org/c_interface.html. DBD::SQLite switched from > using > sqlite_exec, to using the replacement functions of sqlite_compile, > sqlite_step and sqlite_finalize. Your problem was actually one > statement locking the database and not releasing the lock in time. > The
Ok, this is the main problem, that the database/table is locked when I declare the STH. Note that in my example I have 2 sth, the 1st is for insertion and the 2nd just make a select to get the name of the columns. So, the process is (all over the same table): 1 STH1 -> declared (INSERT INTO...) 2 STH2 -> declared (SELECT * FROM...) 3 STH2 -> executed 4 STH2 -> is hloded in a cache table just in case to need to be used in the future. 5 STH1 -> executed (GOT ERROR!) Note that if I don't cache STH2 (step 4) I won't get the error for STH1. So, STH2 is locking the table?! First, STH2 is not a insertion comand, is just a select and don't need lock. Second, after execute the command the table should be freed! The main idea to hold a STH, actually to exists STH in DBI, is to avoid the compilation of the SQL command each time that we need to make the same interaction. So, we really need to keep cache of STH working for SQLite or we will lose desempenho (without talk aboute resources). Show quoted text
> second problem was that the error message that is returned is > incorrect. > The attached patch contains a quick patch that, when applied, will > produce the following output from your sample program:
Thanks for the patch, but this is just fixing the alert message, and not the locking error that is not compatible with the DBI architecture. Show quoted text
> ______________________________________ > LOOP 1 > ______________________________________ > LOOP 2 > DBD::SQLite::st execute failed: database table is locked [for > Statement > "INSERT INTO words VALUES (?,?,?)"] at ./SQLITE-ERROR.pl line 27. > ______________________________________ > LOOP 3 > DBD::SQLite::st execute failed: database table is locked [for > Statement > "INSERT INTO words VALUES (?,?,?)"] at ./SQLITE-ERROR.pl line 27. > > I hope this is helpful. > Uru > -Dave >
Regards, Graciliano M. P.
[GMPASSOS - Sat May 22 19:16:46 2004]: [snip] Show quoted text
> So, STH2 is locking the table?! First, STH2 is not a insertion comand, > is just a select and don't need lock. Second, after execute the > command > the table should be freed! The main idea to hold a STH, actually to > exists STH in DBI, is to avoid the compilation of the SQL command each > time that we need to make the same interaction. So, we really need to > keep cache of STH working for SQLite or we will lose desempenho > (without talk aboute resources).
I should have also mentioned that currently, DBD::SQLite starts a transaction for _every_ statement, including when it is a SELECT type statement. As you point out, this is unnecessary. If you check out Bug 5568 at http://rt.cpan.org/NoAuth/Bug.html?id=5568 you will find a patch from chris@ex-parrot.com that is designed to fix this issue. However, i haven't tried it myself yet, so good luck and let us know how it goes!!! :) Uru -Dave
The path have worked and seems that will not create problems, since it only add a new attribute/flag at imp_sth, that show if it's a select command or not. My question is, DBD::SQLite should hanlde by it self the transactions, or force the user to always use a transaction? With this architecture (from version 0.31+) I always need to be handling 1 transaction, I can't interpolate different process! I think that some option to enable or disable this process at runtime need to be added. So, will be possible to use automatic transanctions or not. About the patch, it can be added to DBD::SQLite, since we can see by the code that it won't create problems and is a simple fix. I actually strongly recommend to include this path now to DBD::SQLite, since by definition a consultation is not an operation that change data in the database and is not defined as a transaction by the design pattners. It will only be a transaction if we have multiple selections with references between them, that some change between this selections can make the final selection invalid. But in our case we have a unique selection, so, definitely is not a transaction. Show quoted text
> [GMPASSOS - Sat May 22 19:16:46 2004]: > > [snip] >
> > So, STH2 is locking the table?! First, STH2 is not a insertion
> comand,
> > is just a select and don't need lock. Second, after execute the > > command > > the table should be freed! The main idea to hold a STH, actually to > > exists STH in DBI, is to avoid the compilation of the SQL command
> each
> > time that we need to make the same interaction. So, we really need
> to
> > keep cache of STH working for SQLite or we will lose desempenho > > (without talk aboute resources).
> > I should have also mentioned that currently, DBD::SQLite starts a > transaction for _every_ statement, including when it is a SELECT type > statement. As you point out, this is unnecessary. If you check out > Bug > 5568 at http://rt.cpan.org/NoAuth/Bug.html?id=5568 you will find a > patch > from chris@ex-parrot.com that is designed to fix this issue. However, > i > haven't tried it myself yet, so good luck and let us know how it > goes!!! :) > > Uru > -Dave
Other thing, the patch own't fix the bug that I have reported! I'm still unable to do this process: 1 STH1 -> declared (INSERT INTO...) 2 STH2 -> declared (SELECT * FROM...) 3 STH2 -> executed 4 STH2 -> is holded in a cache table just in case to need to be used in the future. 5 STH1 -> executed (GOT ERROR!) A simple way to see that I still have this error is to disable autocommit, since we can see in this line at DBD-SQLite-0.31/dbdimp.c Show quoted text
> if ( (!DBIc_is(imp_dbh, DBIcf_AutoCommit)) && (!imp_dbh- >in_tran) ) {
[guest - Sat May 22 23:06:27 2004]: Show quoted text
> [GMPASSOS - Sat May 22 19:16:46 2004]: > > [snip] >
> > So, STH2 is locking the table?! First, STH2 is not a insertion
> comand,
> > is just a select and don't need lock. Second, after execute the > > command > > the table should be freed! The main idea to hold a STH, actually to > > exists STH in DBI, is to avoid the compilation of the SQL command
> each
> > time that we need to make the same interaction. So, we really need
> to
> > keep cache of STH working for SQLite or we will lose desempenho > > (without talk aboute resources).
> > I should have also mentioned that currently, DBD::SQLite starts a > transaction for _every_ statement, including when it is a SELECT type > statement. As you point out, this is unnecessary. If you check out > Bug > 5568 at http://rt.cpan.org/NoAuth/Bug.html?id=5568 you will find a > patch > from chris@ex-parrot.com that is designed to fix this issue. However, > i > haven't tried it myself yet, so good luck and let us know how it > goes!!! :) > > Uru > -Dave
To apply the patch on windows with VS C++ 6+ you need to add the function strncasecmp to the source. Here's the code for that compatible with gcc: #ifndef HAVE_STRNCASECMP #include <string.h> #include <ctype.h> int strncasecmp(const char *s1, const char *s2, unsigned int n) { if (n == 0) return 0; while ((n-- != 0) && (tolower(*(unsigned char *)s1) == tolower(*(unsigned char *)s2))) { if (n == 0 || *s1 == '\0' || *s2 == '\0') return 0; s1++; s2++; } return tolower(*(unsigned char *) s1) - tolower(*(unsigned char *) s2); } #endif [GMPASSOS - Mon May 24 23:22:46 2004]: Show quoted text
> The path have worked and seems that will not create problems, since it > only add a new attribute/flag at imp_sth, that show if it's a select > command or not. > > My question is, DBD::SQLite should hanlde by it self the transactions, > or force the user to always use a transaction? > > With this architecture (from version 0.31+) I always need to be > handling 1 transaction, I can't interpolate different process! > > I think that some option to enable or disable this process at runtime > need to be added. So, will be possible to use automatic transanctions > or not. > > About the patch, it can be added to DBD::SQLite, since we can see by > the code that it won't create problems and is a simple fix. I actually > strongly recommend to include this path now to DBD::SQLite, since by > definition a consultation is not an operation that change data in the > database and is not defined as a transaction by the design pattners. > It > will only be a transaction if we have multiple selections with > references between them, that some change between this selections can > make the final selection invalid. But in our case we have a unique > selection, so, definitely is not a transaction. >
> > [GMPASSOS - Sat May 22 19:16:46 2004]: > > > > [snip] > >
> > > So, STH2 is locking the table?! First, STH2 is not a insertion
> > comand,
> > > is just a select and don't need lock. Second, after execute the > > > command > > > the table should be freed! The main idea to hold a STH, actually
> to
> > > exists STH in DBI, is to avoid the compilation of the SQL command
> > each
> > > time that we need to make the same interaction. So, we really need
> > to
> > > keep cache of STH working for SQLite or we will lose desempenho > > > (without talk aboute resources).
> > > > I should have also mentioned that currently, DBD::SQLite starts a > > transaction for _every_ statement, including when it is a SELECT
> type
> > statement. As you point out, this is unnecessary. If you check out > > Bug > > 5568 at http://rt.cpan.org/NoAuth/Bug.html?id=5568 you will find a > > patch > > from chris@ex-parrot.com that is designed to fix this issue.
> However,
> > i > > haven't tried it myself yet, so good luck and let us know how it > > goes!!! :) > > > > Uru > > -Dave
> >
From: Chris Lightfoot <chris [...] ex-parrot.com
You can use _strnicmp in Visual C/C++, rather than strncasecmp. The prototype is the same.
On Sat May 22 19:16:46 2004, GMPASSOS wrote: Show quoted text
> [guest - Sat May 22 06:23:25 2004]: >
> > G'day Graciliano, > > Thank you for the excellent bug report. I can only hope you find this > > reply as useful. The feature that changed in the versions you > > mentioned > > is the method for retrieving data from the database. A full > > discussion > > on the old and new methods for retrieving data can be found at > > http://www.sqlite.org/c_interface.html. DBD::SQLite switched from > > using > > sqlite_exec, to using the replacement functions of sqlite_compile, > > sqlite_step and sqlite_finalize. Your problem was actually one > > statement locking the database and not releasing the lock in time. > > The
> > Ok, this is the main problem, that the database/table is locked when I > declare the STH. Note that in my example I have 2 sth, the 1st is for > insertion and the 2nd just make a select to get the name of the > columns. So, the process is (all over the same table): > > 1 STH1 -> declared (INSERT INTO...) > 2 STH2 -> declared (SELECT * FROM...) > 3 STH2 -> executed > 4 STH2 -> is hloded in a cache table just in case to need to be used in > the future. > 5 STH1 -> executed (GOT ERROR!) > > Note that if I don't cache STH2 (step 4) I won't get the error for STH1. > > So, STH2 is locking the table?! First, STH2 is not a insertion comand, > is just a select and don't need lock. Second, after execute the command > the table should be freed! The main idea to hold a STH, actually to > exists STH in DBI, is to avoid the compilation of the SQL command each > time that we need to make the same interaction. So, we really need to > keep cache of STH working for SQLite or we will lose desempenho > (without talk aboute resources). >
> > second problem was that the error message that is returned is > > incorrect. > > The attached patch contains a quick patch that, when applied, will > > produce the following output from your sample program:
> > Thanks for the patch, but this is just fixing the alert message, and > not the locking error that is not compatible with the DBI architecture. >
> > ______________________________________ > > LOOP 1 > > ______________________________________ > > LOOP 2 > > DBD::SQLite::st execute failed: database table is locked [for > > Statement > > "INSERT INTO words VALUES (?,?,?)"] at ./SQLITE-ERROR.pl line 27. > > ______________________________________ > > LOOP 3 > > DBD::SQLite::st execute failed: database table is locked [for > > Statement > > "INSERT INTO words VALUES (?,?,?)"] at ./SQLITE-ERROR.pl line 27. > > > > I hope this is helpful. > > Uru > > -Dave > >
> > Regards, > Graciliano M. P.