From c87d34d05d7e6d9421e2b2eddcf51991067286b2 Mon Sep 17 00:00:00 2001 From: danielk1977 Date: Fri, 6 Jan 2006 13:00:28 +0000 Subject: [PATCH] Add some simple tests and fixes for shared-schema locking. (CVS 2869) FossilOrigin-Name: d054bed15aee3edef93cc84c83be443cdd489946 --- manifest | 22 +++++----- manifest.uuid | 2 +- src/btree.c | 14 ++++-- src/btree.h | 3 +- src/prepare.c | 20 +++++++-- test/shared.test | 103 +++++++++++++++++++++++++++++++++++++++++--- www/sharedcache.tcl | 29 ++++++++++--- 7 files changed, 162 insertions(+), 31 deletions(-) diff --git a/manifest b/manifest index 602d9890fb..d51cc5a3df 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Back\sout\sthe\saddition\sof\sthe\snew\sheader\sfile\s-\sbad\sidea.\s(CVS\s2868) -D 2006-01-06T12:03:19 +C Add\ssome\ssimple\stests\sand\sfixes\sfor\sshared-schema\slocking.\s(CVS\s2869) +D 2006-01-06T13:00:29 F Makefile.in 131285a3e97597dd7ed3eb23a1010c7d0685ae41 F Makefile.linux-gcc aee18d8a05546dcf1888bd4547e442008a49a092 F README 9c4e2d6706bdcc3efdd773ce752a8cdab4f90028 @@ -34,8 +34,8 @@ F src/alter.c e9deb3f4fd7c663a0d1f235d541bc5ea1f2cfa8b F src/analyze.c d821684cdb4d0403e327e4a3440a832e9e54fa3a F src/attach.c 999104c56a60b88eab11ef9c8f40dedf1650b287 F src/auth.c cdec356a5cd8b217c346f816c5912221537fe87f -F src/btree.c d1402f4e1cfc500b31d13990f36dd8d3d27443bc -F src/btree.h 96b8c00c6e11ff92f8d3d6a7a0ff358bd10d8f19 +F src/btree.c f34c1e8d93003cbd8cc579865e0c0db4f595b97e +F src/btree.h f7ba8e2f9f387cca4978e1495504a0bf556dcbf2 F src/build.c 715ac7d49bbfcae5f3fdfd60885397b2133c283b F src/callback.c 62066afd516f220575e81b1a1239ab92a2eae252 F src/complete.c df1681cef40dec33a286006981845f87b194e7a4 @@ -63,7 +63,7 @@ F src/pager.c 07509ddb478f5a70f9ff53607ab8a44456c22811 F src/pager.h e0acb095b3ad0bca48f2ab00c87346665643f64f F src/parse.y 58258759fabdd48f1d2561e276097290b1ea2680 F src/pragma.c 4af4041a88d41421b8ff2e5574d82d7b9d1e35b1 -F src/prepare.c 67ff283f7c71e32a91d8c843e758eb4bf68ab94e +F src/prepare.c 48baea211104dfcfa59857a32d62c934f63a15ed F src/printf.c f47a2f4b5387cd2ebb12e9117a1a5d6bd9a2b812 F src/random.c ff5e9a8cad790e2a51cd4d2e7737dc8540e09d1d F src/select.c a60e5c7fad9ce7adc78d9eb32a0a89dd5acd04fb @@ -222,7 +222,7 @@ F test/select4.test c239f516aa31f42f2ef7c6d7cd01105f08f934ca F test/select5.test 07a90ab3c7e3f0a241a9cdea1d997b2c8a89ff0b F test/select6.test f459a19bdac0501c4d3eb1a4df4b7a76f1bb8ad4 F test/select7.test 1bf795b948c133a15a2a5e99d3270e652ec58ce6 -F test/shared.test ee5a4154d257e4c2ce1ae418783b87847473de90 +F test/shared.test eb6b9ac456f60ed485b9ca5873d563abacae815a F test/sort.test 0e4456e729e5a92a625907c63dcdedfbe72c5dc5 F test/subquery.test e6de53332c0301b3cfa34edc3f3cd5fa1e859efd F test/subselect.test 2d13fb7f450db3595adcdd24079a0dd1d2d6abc2 @@ -328,7 +328,7 @@ F www/optimizing.tcl f0b2538988d1bbad16cbfe63ec6e8f48c9eb04e5 F www/optoverview.tcl 815df406a38c9f69b27d37e8f7ede004c6d9f19e F www/pragma.tcl 44f7b665ca598ad24724f35991653638a36a6e3f F www/quickstart.tcl 2f3daf8038e82a102e1e8cc877aafa7a413f5f11 -F www/sharedcache.tcl c42098d1436bcb54ec7f08d07c2e75316e2dde68 +F www/sharedcache.tcl 884a93bf0f814204b24959c0c6229d61dd4734cc F www/speed.tcl 656ed5be8cc9d536353e1a96927b925634a62933 F www/sqlite.tcl a883ed7b47371d31d471e6aea5ed1f972ae8e1be F www/support.tcl 7961ce16290692578d783bb11f0dc8391a9be9c3 @@ -337,7 +337,7 @@ F www/tclsqlite.tcl bb0d1357328a42b1993d78573e587c6dcbc964b9 F www/vdbe.tcl 87a31ace769f20d3627a64fa1fade7fed47b90d0 F www/version3.tcl a99cf5f6d8bd4d5537584a2b342f0fb9fa601d8b F www/whentouse.tcl 97e2b5cd296f7d8057e11f44427dea8a4c2db513 -P 752a2754879becc32da9f9b910f3330f8c7145e4 -R 77860345776bf3dbb42b93b9296c48b1 -U drh -Z 246dc71f2673d21c26bf8f1421f9f9ff +P 0c4c45c36fe1c3c2980155ef5126a2ad53100d65 +R c892910cec71c19aeb323bc85b1a92ea +U danielk1977 +Z 5e6321921a4935029ecd84d938104014 diff --git a/manifest.uuid b/manifest.uuid index 5dfee886b7..c891ff356a 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -0c4c45c36fe1c3c2980155ef5126a2ad53100d65 \ No newline at end of file +d054bed15aee3edef93cc84c83be443cdd489946 \ No newline at end of file diff --git a/src/btree.c b/src/btree.c index 79de82c8d5..c1032f5096 100644 --- a/src/btree.c +++ b/src/btree.c @@ -9,7 +9,7 @@ ** May you share freely, never taking more than you give. ** ************************************************************************* -** $Id: btree.c,v 1.281 2006/01/06 06:33:12 danielk1977 Exp $ +** $Id: btree.c,v 1.282 2006/01/06 13:00:29 danielk1977 Exp $ ** ** This file implements a external (disk-based) database using BTrees. ** For a detailed discussion of BTrees, refer to @@ -601,7 +601,7 @@ static int restoreCursorPosition(BtCursor *pCur, int doSeek){ ** Query to see if btree handle p may obtain a lock of type eLock ** (READ_LOCK or WRITE_LOCK) on the table with root-page iTab. Return ** SQLITE_OK if the lock may be obtained (by calling lockTable()), or -** SQLITE_BUSY if not. +** SQLITE_LOCKED if not. */ static int queryTableLock(Btree *p, Pgno iTab, u8 eLock){ BtShared *pBt = p->pBt; @@ -635,7 +635,7 @@ static int queryTableLock(Btree *p, Pgno iTab, u8 eLock){ for(pIter=pBt->pLock; pIter; pIter=pIter->pNext){ if( pIter->pBtree!=p && pIter->iTable==iTab && (pIter->eLock!=eLock || eLock!=READ_LOCK) ){ - return SQLITE_BUSY; + return SQLITE_LOCKED; } } } @@ -6484,6 +6484,14 @@ void *sqlite3BtreeSchema(Btree *p, int nBytes, void(*xFree)(void *)){ return pBt->pSchema; } +/* +** Return true if another user of the same shared btree as the argument +** handle holds an exclusive lock on the sqlite_master table. +*/ +int sqlite3BtreeSchemaLocked(Btree *p){ + return (queryTableLock(p, MASTER_ROOT, READ_LOCK)!=SQLITE_OK); +} + #ifndef SQLITE_OMIT_SHARED_CACHE /* ** Enable the shared pager and schema features. diff --git a/src/btree.h b/src/btree.h index 69eb34fbf8..6a65813c90 100644 --- a/src/btree.h +++ b/src/btree.h @@ -13,7 +13,7 @@ ** subsystem. See comments in the source code for a detailed description ** of what each interface routine does. ** -** @(#) $Id: btree.h,v 1.67 2006/01/05 11:34:34 danielk1977 Exp $ +** @(#) $Id: btree.h,v 1.68 2006/01/06 13:00:30 danielk1977 Exp $ */ #ifndef _BTREE_H_ #define _BTREE_H_ @@ -77,6 +77,7 @@ int sqlite3BtreeIsInTrans(Btree*); int sqlite3BtreeIsInStmt(Btree*); int sqlite3BtreeSync(Btree*, const char *zMaster); void *sqlite3BtreeSchema(Btree *, int, void(*)(void *)); +int sqlite3BtreeSchemaLocked(Btree *); const char *sqlite3BtreeGetFilename(Btree *); const char *sqlite3BtreeGetDirname(Btree *); diff --git a/src/prepare.c b/src/prepare.c index 6a75aa2ffd..564aff53b8 100644 --- a/src/prepare.c +++ b/src/prepare.c @@ -13,7 +13,7 @@ ** interface, and routines that contribute to loading the database schema ** from disk. ** -** $Id: prepare.c,v 1.13 2006/01/05 11:34:34 danielk1977 Exp $ +** $Id: prepare.c,v 1.14 2006/01/06 13:00:30 danielk1977 Exp $ */ #include "sqliteInt.h" #include "os.h" @@ -436,8 +436,9 @@ int sqlite3_prepare( Parse sParse; char *zErrMsg = 0; int rc = SQLITE_OK; + int i; - assert(!sqlite3Tsd()->mallocFailed); + assert( !sqlite3Tsd()->mallocFailed ); assert( ppStmt ); *ppStmt = 0; @@ -445,6 +446,19 @@ int sqlite3_prepare( return SQLITE_MISUSE; } + /* If any attached database schemas are locked, do not proceed with + ** compilation. Instead return SQLITE_LOCKED immediately. + */ + for(i=0; inDb; i++) { + Btree *pBt = db->aDb[i].pBt; + if( pBt && sqlite3BtreeSchemaLocked(pBt) ){ + const char *zDb = db->aDb[i].zName; + sqlite3Error(db, SQLITE_LOCKED, "database schema is locked: %s", zDb); + sqlite3SafetyOff(db); + return SQLITE_LOCKED; + } + } + memset(&sParse, 0, sizeof(sParse)); sParse.db = db; sqlite3RunParser(&sParse, zSql, &zErrMsg); @@ -524,7 +538,7 @@ int sqlite3_prepare16( ** tricky bit is figuring out the pointer to return in *pzTail. */ char *zSql8 = 0; - char *zTail8 = 0; + const char *zTail8 = 0; int rc; if( sqlite3SafetyCheck(db) ){ diff --git a/test/shared.test b/test/shared.test index cc32aa0190..746cfe478c 100644 --- a/test/shared.test +++ b/test/shared.test @@ -11,7 +11,7 @@ # This file implements regression tests for SQLite library. The # focus of this file is testing the SELECT statement. # -# $Id: shared.test,v 1.3 2006/01/06 06:33:13 danielk1977 Exp $ +# $Id: shared.test,v 1.4 2006/01/06 13:00:30 danielk1977 Exp $ set testdir [file dirname $argv0] source $testdir/tester.tcl @@ -73,7 +73,7 @@ do_test shared-1.4 { catchsql { INSERT INTO abc VALUES(4, 5, 6); } db2 -} {1 {database is locked}} +} {1 {database table is locked}} do_test shared-1.5 { # Using connection 2 (the one without the open transaction), try to create # a new table. This should fail because of the open read transaction @@ -81,7 +81,7 @@ do_test shared-1.5 { catchsql { CREATE TABLE def(d, e, f); } db2 -} {1 {database is locked}} +} {1 {database table is locked}} do_test shared-1.6 { # Upgrade connection 1's transaction to a write transaction. Create # a new table - def - and insert a row into it. Because the connection 1 @@ -104,7 +104,7 @@ do_test shared-1.7 { catchsql { SELECT * FROM sqlite_master; } db2 -} {1 {database is locked}} +} {1 {database schema is locked: main}} do_test shared-1.8 { # Commit the connection 1 transaction. execsql { @@ -133,7 +133,7 @@ do_test shared-2.2 { catchsql { INSERT INTO abc VALUES(1, 2, 3); } db2 -} {1 {database is locked}} +} {1 {database table is locked}} do_test shared-2.3 { # Turn db's transaction into a write-transaction. db3 should still be # able to read from table def (but will not see the new row). Connection @@ -157,7 +157,7 @@ do_test shared-2.3 { ] [ catchsql { SELECT * FROM def; } db2 ] -} {0 {IV V VI} 1 {database is locked}} +} {0 {IV V VI} 1 {database table is locked}} do_test shared-2.4 { # Commit the open transaction on db. db2 still holds a read-transaction. # This should prevent db3 from writing to the database, but not from @@ -257,10 +257,16 @@ do_test shared-4.1.3 { set sqlite_open_file_count } {2} +# Sanity check: Create a table in ./test.db via handle db, and test that handle +# db2 can "see" the new table immediately. A handle using a seperate pager +# cache would have to reload the database schema before this were possible. +# do_test shared-4.2.1 { execsql { CREATE TABLE abc(a, b, c); + CREATE TABLE def(d, e, f); INSERT INTO abc VALUES('i', 'ii', 'iii'); + INSERT INTO def VALUES('I', 'II', 'III'); } } {} do_test shared-4.2.2 { @@ -269,6 +275,91 @@ do_test shared-4.2.2 { } db2 } {i ii iii} +# Open a read-transaction and read from table abc via handle 2. Check that +# handle 1 can read table abc. Check that handle 1 cannot modify table abc +# or the database schema. Then check that handle 1 can modify table def. +# +do_test shared-4.3.1 { + execsql { + BEGIN; + SELECT * FROM test.abc; + } db2 +} {i ii iii} +do_test shared-4.3.2 { + catchsql { + INSERT INTO abc VALUES('iv', 'v', 'vi'); + } +} {1 {database table is locked}} +do_test shared-4.3.3 { + catchsql { + CREATE TABLE ghi(g, h, i); + } +} {1 {database table is locked}} +do_test shared-4.3.3 { + catchsql { + INSERT INTO def VALUES('IV', 'V', 'VI'); + } +} {0 {}} +do_test shared-4.3.4 { + # Cleanup: commit the transaction opened by db2. + execsql { + COMMIT + } db2 +} {} + +# Open a write-transaction using handle 1 and modify the database schema. +# Then try to execute a compiled statement to read from the same +# database via handle 2 (fails to get the lock on sqlite_master). Also +# try to compile a read of the same database using handle 2 (also fails). +# Finally, compile a read of the other database using handle 2. This +# should also fail. +# +do_test shared-4.4.1.2 { + # Sanity check 1: Check that the schema is what we think it is when viewed + # via handle 1. + execsql { + CREATE TABLE test2.ghi(g, h, i); + SELECT 'test.db:'||name FROM sqlite_master + UNION ALL + SELECT 'test2.db:'||name FROM test2.sqlite_master; + } +} {test.db:abc test.db:def test2.db:ghi} +do_test shared-4.4.1.2 { + # Sanity check 2: Check that the schema is what we think it is when viewed + # via handle 2. + execsql { + SELECT 'test2.db:'||name FROM sqlite_master + UNION ALL + SELECT 'test.db:'||name FROM test.sqlite_master; + } db2 +} {test2.db:ghi test.db:abc test.db:def} + +do_test shared-4.4.2 { + set ::DB2 [sqlite3_connection_pointer db2] + set sql {SELECT * FROM abc} + set ::STMT1 [sqlite3_prepare $::DB2 $sql -1 DUMMY] + execsql { + BEGIN; + CREATE TABLE jkl(j, k, l); + } + sqlite3_step $::STMT1 +} {SQLITE_ERROR} +do_test shared-4.4.3 { + sqlite3_finalize $::STMT1 +} {SQLITE_LOCKED} +do_test shared-4.4.4 { + set rc [catch { + set ::STMT1 [sqlite3_prepare $::DB2 $sql -1 DUMMY] + } msg] + list $rc $msg +} {1 {(6) database schema is locked: test}} +do_test shared-4.4.5 { + set rc [catch { + set ::STMT1 [sqlite3_prepare $::DB2 "SELECT * FROM ghi" -1 DUMMY] + } msg] + list $rc $msg +} {1 {(6) database schema is locked: test}} + catch {db2 close} catch {db close} diff --git a/www/sharedcache.tcl b/www/sharedcache.tcl index a1f95ac7e7..a8dca590c6 100644 --- a/www/sharedcache.tcl +++ b/www/sharedcache.tcl @@ -120,12 +120,9 @@ read-lock, write-lock or no lock on each database table. or a single active write lock. To read data a table, a connection must first obtain a read-lock. To write to a table, a connection must obtain a write-lock on that table. If a required table lock cannot be obtained, -the query fails and SQLITE_BUSY is returned to the caller. +the query fails and SQLITE_LOCKED is returned to the caller.

-

TODO: Should we be invoking the busy-handler here? Just waiting won't do -any good, but something else might...

-

Once a connection obtains a table lock, it is not released until the current transaction (read or write) is concluded.

@@ -181,12 +178,32 @@ accessing any database tables or obtaining any other read or write locks. a CREATE or DROP TABLE statement), a connection must obtain a write-lock on sqlite_master. -
  • A connection may not compile an SQL statement that refers to database -tables if any other connection is holding a write-lock on sqlite_master. +
  • A connection may not compile an SQL statement if any other connection +is holding a write-lock on the sqlite_master table of any attached +database (including the default database, "main").
    • } +HEADING 3 {Schema locking and attached databases} + +puts { +

    The final point in the bullet list is deceptively complicated when +multiple databases are attached to connections. Exactly when is access to a +specific database schema "required" to compile a statement? The way in +which SQLite resolves the names of schema objects (i.e. tables, indices, +triggers and views) depends on whether or not the name was qualified +or unqualified in the original SQL statement. The first statement below +uses a qualified table name, the second uses an unqualified table name. +Both refer to the same underlying table. +

    +
    +    SELECT name FROM main.sqlite_master;
+    SELECT name FROM sqlite_master;
+
    +1 +} + HEADING 1 {Thread Related Issues} puts { -- 2.47.2