From e6efa74b9f8fca9f6baafc33d7091ae282079001 Mon Sep 17 00:00:00 2001 From: danielk1977 Date: Wed, 10 Nov 2004 11:55:10 +0000 Subject: [PATCH] Ensure tables cannot be created/dropped when btree cursors are open. (CVS 2085) FossilOrigin-Name: 8e5c2e5df8b824f7efb27e776240f005c6f1f0ff --- manifest | 24 +++++++-------- manifest.uuid | 2 +- src/btree.c | 33 ++++++++++++++++----- src/build.c | 7 +++-- src/date.c | 10 ++++--- src/test3.c | 3 +- src/vdbe.c | 23 +++++++++------ test/btree.test | 6 ++-- test/table.test | 77 +++++++++++++++++++++++++++++++++++++++++++++++-- 9 files changed, 142 insertions(+), 43 deletions(-) diff --git a/manifest b/manifest index fc9688b403..100fd6245a 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Add\suser\sdocumentation\sfor\sthe\s"pragma\sauto_vacuum"\scommand.\s(CVS\s2084) -D 2004-11-10T05:48:57 +C Ensure\stables\scannot\sbe\screated/dropped\swhen\sbtree\scursors\sare\sopen.\s(CVS\s2085) +D 2004-11-10T11:55:11 F Makefile.in c4d2416860f472a1e3393714d0372074197565df F Makefile.linux-gcc a9e5a0d309fa7c38e7c14d3ecf7690879d3a5457 F README a01693e454a00cc117967e3f9fdab2d4d52e9bc1 @@ -29,10 +29,10 @@ F sqlite3.def dbaeb20c153e1d366e8f421b55a573f5dfc00863 F sqlite3.pc.in 985b9bf34192a549d7d370e0f0b6b34a4f61369a F src/attach.c e49d09dad9f5f9fb10b4b0c1be5a70ae4c45e689 F src/auth.c 3b81f2a42f48a62c2c9c9b0eda31a157c681edea -F src/btree.c 63a84350a18f6ca68f16e2a12018b5041444a2df +F src/btree.c 9fd74df65bad768a441afefc3b73174d45b85d5b F src/btree.h 861e40b759a195ba63819740e484390012cf81ab -F src/build.c d623d84fd7f4e9cc0c5e8d1b96aab7886cf0f84d -F src/date.c fcbade133371925ac28fe3f926031d60b07e2474 +F src/build.c 27e3cc9e5d5187fa6f23d2e60b76e82c4bc0e7b0 +F src/date.c 21413a0fc3962e2f18ba471883bcc35ddd7ff314 F src/delete.c f0af21a1ede15524a5edd59fe10ef486283a1ee9 F src/expr.c 5f9afecf27e048b8f3627b5a9be3136bc1d9bdf1 F src/func.c 600e506bccf7648df8ad03efb417560d0f7ad4c1 @@ -66,7 +66,7 @@ F src/table.c 25b3ff2b39b7d87e8d4a5da0713d68dfc06cbee9 F src/tclsqlite.c 0302e3f42f015d132d1291f3388c06e86c24a008 F src/test1.c 91345097d94b4ad71f88776c2764e18c7955502a F src/test2.c b11fa244fff02190707dd0879987c37c75e61fc8 -F src/test3.c de9edf178c02707cd37fd80b54e4c2ea77251cc0 +F src/test3.c 6f1ec93e13632a004b527049535079eda84c459d F src/test4.c 7c6b9fc33dd1f3f93c7f1ee6e5e6d016afa6c1df F src/test5.c b001fa7f1b9e2dc5c2331de62fc641b5ab2bd7a1 F src/tokenize.c c48221284e729be067237a8cfd7848fb62ee4a92 @@ -75,7 +75,7 @@ F src/update.c 3cc67f6053495152e82a6a48c93ed331218e936e F src/utf.c f4f83acd73389090e32d6589d307fc55d794c7ed F src/util.c 005fdf2d008f3429d081766ad6098fdd86d8d8e6 F src/vacuum.c ecb4a2c6f1ac5cc9b394dc64d3bb14ca650c4f60 -F src/vdbe.c b324acdaff3b1f21cd369bbb0df30e1c3fa828f4 +F src/vdbe.c 8bc549adf596ee792facd229ee7f7bfc0b690ad4 F src/vdbe.h 067ca8d6750ba4f69a50284765e5883dee860181 F src/vdbeInt.h 6017100adff362b8dfa37a69e3f1431f084bfa5b F src/vdbeapi.c 3965bf4678ae32c05f73550c1b5be3268f9f3006 @@ -93,7 +93,7 @@ F test/bigfile.test d3744a8821ce9abb8697f2826a3e3d22b719e89f F test/bigrow.test f0aeb7573dcb8caaafea76454be3ade29b7fc747 F test/bind.test fa74f98417cd313f28272acff832a8a7d04a0916 F test/blob.test fc41fe95bdc10da51f0dee73ce86e75ce1d6eb9d -F test/btree.test ac0e3327d09ca3daace57aefb7423d1766d2bcfd +F test/btree.test 3a734c9e6796616530ccbc84444241ddae595dac F test/btree2.test aa4a6d05b1ea90b1acaf83ba89039dd302a88635 F test/btree4.test 3797b4305694c7af6828675b0f4b1424b8ca30e4 F test/btree5.test 8e5ff32c02e685d36516c6499add9375fe1377f2 @@ -173,7 +173,7 @@ F test/select6.test 4ce9fa563662d5b2f5a8ff57e4d8b2f5cd186d38 F test/select7.test c71c822a82c80bbd55558b4b69d35442dfe22ffd F test/sort.test c97c1a3289337b1dc349ac8a59e0780d2dcfd90b F test/subselect.test 50f98723f00e97b1839d36410ee63597ca82d775 -F test/table.test 0d7659fa9ffd5946c7d2f006b4c8608d7e9cc518 +F test/table.test 87a6219c784722249a2f604b6495ce171ca2588a F test/tableapi.test b21ab097e87a5484bb61029e69e1a4e5c5e65ede F test/tclsqlite.test 5e262df81a638a058536fb6d6666f316843ac7b2 F test/temptable.test 63a16e3ad19adf073cfbcdf7624c92ac5236522c @@ -255,7 +255,7 @@ F www/tclsqlite.tcl 560ecd6a916b320e59f2917317398f3d59b7cc25 F www/vdbe.tcl 095f106d93875c94b47367384ebc870517431618 F www/version3.tcl 092a01f5ef430d2c4acc0ae558d74c4bb89638a0 F www/whentouse.tcl fdacb0ba2d39831e8a6240d05a490026ad4c4e4c -P f81b9c1c022772378aad32ec45d0027beeb36574 -R bfd59cfe949547755a65241f31d7d209 +P fe200eaf373998574cc059086bfc93d6c44ec5a3 +R 5c28b72443e2b85deec95ab22d09127a U danielk1977 -Z e40615b74d37b060e4ae6d8ae3079d56 +Z 1b8b884c02278a9e78ec1e566c672bd7 diff --git a/manifest.uuid b/manifest.uuid index 151fdfd004..3b75a6870f 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -fe200eaf373998574cc059086bfc93d6c44ec5a3 \ No newline at end of file +8e5c2e5df8b824f7efb27e776240f005c6f1f0ff \ No newline at end of file diff --git a/src/btree.c b/src/btree.c index d751f35ff4..2e086b2f90 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.215 2004/11/08 09:26:09 danielk1977 Exp $ +** $Id: btree.c,v 1.216 2004/11/10 11:55:11 danielk1977 Exp $ ** ** This file implements a external (disk-based) database using BTrees. ** For a detailed discussion of BTrees, refer to @@ -4304,11 +4304,17 @@ int sqlite3BtreeDelete(BtCursor *pCur){ } rc = sqlite3pager_write(pPage->aData); if( rc ) return rc; + + /* Locate the cell within it's page and leave pCell pointing to the + ** data. The clearCell() call frees any overflow pages associated with the + ** cell. The cell itself is still intact. + */ pCell = findCell(pPage, pCur->idx); if( !pPage->leaf ){ pgnoChild = get4byte(pCell); } clearCell(pPage, pCell); + if( !pPage->leaf ){ /* ** The entry we are about to delete is not a leaf so if we do not @@ -4375,7 +4381,6 @@ int sqlite3BtreeCreateTable(Btree *pBt, int *piTable, int flags){ MemPage *pRoot; Pgno pgnoRoot; int rc; -/* TODO: Disallow schema modifications if there are open cursors */ if( pBt->inTrans!=TRANS_WRITE ){ /* Must start a transaction first */ return pBt->readOnly ? SQLITE_READONLY : SQLITE_ERROR; @@ -4383,6 +4388,16 @@ int sqlite3BtreeCreateTable(Btree *pBt, int *piTable, int flags){ if( pBt->readOnly ){ return SQLITE_READONLY; } + + /* It is illegal to create a table if any cursors are open on the + ** database. This is because in auto-vacuum mode the backend may + ** need to move a database page to make room for the new root-page. + ** If an open cursor was using the page a problem would occur. + */ + if( pBt->pCursor ){ + return SQLITE_LOCKED; + } + #ifdef SQLITE_OMIT_AUTOVACUUM rc = allocatePage(pBt, &pRoot, &pgnoRoot, 1, 0); if( rc ) return rc; @@ -4565,17 +4580,19 @@ int sqlite3BtreeClearTable(Btree *pBt, int iTable){ int sqlite3BtreeDropTable(Btree *pBt, int iTable, int *piMoved){ int rc; MemPage *pPage = 0; - BtCursor *pCur; if( pBt->inTrans!=TRANS_WRITE ){ return pBt->readOnly ? SQLITE_READONLY : SQLITE_ERROR; } -/* TODO: Disallow schema modifications if there are open cursors */ - for(pCur=pBt->pCursor; pCur; pCur=pCur->pNext){ - if( pCur->pgnoRoot==(Pgno)iTable ){ - return SQLITE_LOCKED; /* Cannot drop a table that has a cursor */ - } + /* It is illegal to drop a table if any cursors are open on the + ** database. This is because in auto-vacuum mode the backend may + ** need to move another root-page to fill a gap left by the deleted + ** root page. If an open cursor was using this page a problem would + ** occur. + */ + if( pBt->pCursor ){ + return SQLITE_LOCKED; } rc = getPage(pBt, (Pgno)iTable, &pPage); diff --git a/src/build.c b/src/build.c index 236cbea64c..9dc6db2014 100644 --- a/src/build.c +++ b/src/build.c @@ -22,7 +22,7 @@ ** COMMIT ** ROLLBACK ** -** $Id: build.c,v 1.274 2004/11/09 12:44:38 danielk1977 Exp $ +** $Id: build.c,v 1.275 2004/11/10 11:55:12 danielk1977 Exp $ */ #include "sqliteInt.h" #include @@ -758,6 +758,7 @@ void sqlite3StartTable( sqlite3VdbeAddOp(v, OP_Dup, 0, 0); sqlite3VdbeAddOp(v, OP_String8, 0, 0); sqlite3VdbeAddOp(v, OP_PutIntKey, 0, 0); + sqlite3VdbeAddOp(v, OP_Close, 0, 0); } } @@ -1380,6 +1381,8 @@ void sqlite3EndTable(Parse *pParse, Token *pEnd, Select *pSelect){ v = sqlite3GetVdbe(pParse); if( v==0 ) return; + sqlite3VdbeAddOp(v, OP_Close, 0, 0); + /* Create the rootpage for the new table and push it onto the stack. ** A view has no rootpage, so just push a zero onto the stack for ** views. Initialize zType at the same time. @@ -1396,8 +1399,6 @@ void sqlite3EndTable(Parse *pParse, Token *pEnd, Select *pSelect){ zType2 = "VIEW"; } - sqlite3VdbeAddOp(v, OP_Close, 0, 0); - /* If this is a CREATE TABLE xx AS SELECT ..., execute the SELECT ** statement to populate the new table. The root-page number for the ** new table is on the top of the vdbe stack. diff --git a/src/date.c b/src/date.c index 3dff942af0..a65a25d5bd 100644 --- a/src/date.c +++ b/src/date.c @@ -16,7 +16,7 @@ ** sqlite3RegisterDateTimeFunctions() found at the bottom of the file. ** All other code has file scope. ** -** $Id: date.c,v 1.39 2004/11/09 16:13:33 danielk1977 Exp $ +** $Id: date.c,v 1.40 2004/11/10 11:55:12 danielk1977 Exp $ ** ** NOTES: ** @@ -938,7 +938,6 @@ static void currentTimeFunc( time_t t; char *zFormat = (char *)sqlite3_user_data(context); char zBuf[20]; - struct tm now; #ifdef SQLITE_TEST /* This test variable is located in os_XXX.c */ @@ -950,8 +949,11 @@ extern int sqlite3_current_time; t = sqlite3_current_time; } #endif - localtime_r(&t, &now); - strftime(zBuf, 20, zFormat, &now); + + sqlite3OsEnterMutex(); + strftime(zBuf, 20, zFormat, gmtime(&t)); + sqlite3OsLeaveMutex(); + sqlite3_result_text(context, zBuf, -1, SQLITE_TRANSIENT); } #endif diff --git a/src/test3.c b/src/test3.c index da7ce0e7d3..0ffcf93262 100644 --- a/src/test3.c +++ b/src/test3.c @@ -13,7 +13,7 @@ ** is not included in the SQLite library. It is used for automated ** testing of the SQLite library. ** -** $Id: test3.c,v 1.56 2004/11/05 00:43:12 drh Exp $ +** $Id: test3.c,v 1.57 2004/11/10 11:55:12 danielk1977 Exp $ */ #include "sqliteInt.h" #include "pager.h" @@ -44,6 +44,7 @@ static char *errorName(int rc){ case SQLITE_CANTOPEN: zName = "SQLITE_CANTOPEN"; break; case SQLITE_PROTOCOL: zName = "SQLITE_PROTOCOL"; break; case SQLITE_EMPTY: zName = "SQLITE_EMPTY"; break; + case SQLITE_LOCKED: zName = "SQLITE_LOCKED"; break; default: zName = "SQLITE_Unknown"; break; } return zName; diff --git a/src/vdbe.c b/src/vdbe.c index bb2106c2f1..b00d410e51 100644 --- a/src/vdbe.c +++ b/src/vdbe.c @@ -43,7 +43,7 @@ ** in this file for details. If in doubt, do not deviate from existing ** commenting and indentation practices when changing or adding code. ** -** $Id: vdbe.c,v 1.424 2004/11/05 00:43:12 drh Exp $ +** $Id: vdbe.c,v 1.425 2004/11/10 11:55:13 danielk1977 Exp $ */ #include "sqliteInt.h" #include "os.h" @@ -3646,15 +3646,20 @@ case OP_IdxIsNull: { */ case OP_Destroy: { int iMoved; - rc = sqlite3BtreeDropTable(db->aDb[pOp->p2].pBt, pOp->p1, &iMoved); - pTos++; - pTos->flags = MEM_Int; - pTos->i = iMoved; -#ifndef SQLITE_OMIT_AUTOVACUUM - if( iMoved!=0 ){ - sqlite3RootPageMoved(&db->aDb[pOp->p2], iMoved, pOp->p1); + if( db->activeVdbeCnt>1 ){ + rc = SQLITE_LOCKED; + }else{ + assert( db->activeVdbeCnt==1 ); + rc = sqlite3BtreeDropTable(db->aDb[pOp->p2].pBt, pOp->p1, &iMoved); + pTos++; + pTos->flags = MEM_Int; + pTos->i = iMoved; + #ifndef SQLITE_OMIT_AUTOVACUUM + if( rc==SQLITE_OK && iMoved!=0 ){ + sqlite3RootPageMoved(&db->aDb[pOp->p2], iMoved, pOp->p1); + } + #endif } -#endif break; } diff --git a/test/btree.test b/test/btree.test index 3657ee8bfa..fe10561b2c 100644 --- a/test/btree.test +++ b/test/btree.test @@ -11,7 +11,7 @@ # This file implements regression tests for SQLite library. The # focus of this script is btree database backend # -# $Id: btree.test,v 1.31 2004/10/18 21:34:47 drh Exp $ +# $Id: btree.test,v 1.32 2004/11/10 11:55:14 danielk1977 Exp $ set testdir [file dirname $argv0] @@ -467,6 +467,7 @@ do_test btree-6.6.1 { lindex [btree_pager_stats $::b1] 1 } {1} do_test btree-6.7 { + btree_close_cursor $::c1 btree_drop_table $::b1 $::t2 } {} do_test btree-6.7.1 { @@ -491,8 +492,9 @@ do_test btree-6.9.1 { # If we drop table 1 it just clears the table. Table 1 always exists. # do_test btree-6.10 { - btree_close_cursor $::c1 + btree_close_cursor $::c2 btree_drop_table $::b1 1 + set ::c2 [btree_cursor $::b1 $::t2 1] set ::c1 [btree_cursor $::b1 1 1] btree_first $::c1 btree_eof $::c1 diff --git a/test/table.test b/test/table.test index a30529447f..9069a5f343 100644 --- a/test/table.test +++ b/test/table.test @@ -11,7 +11,7 @@ # This file implements regression tests for SQLite library. The # focus of this file is testing the CREATE TABLE statement. # -# $Id: table.test,v 1.32 2004/11/09 16:13:33 danielk1977 Exp $ +# $Id: table.test,v 1.33 2004/11/10 11:55:15 danielk1977 Exp $ set testdir [file dirname $argv0] source $testdir/tester.tcl @@ -524,6 +524,9 @@ do_test table-12.2 { } } {{CREATE TABLE t8(b number(5,10),h,i integer,j BLOB)}} +#-------------------------------------------------------------------- +# Test cases table-13.* +# # Test the ability to have default values of CURRENT_TIME, CURRENT_DATE # and CURRENT_TIMESTAMP. # @@ -546,8 +549,8 @@ foreach {date time} { 2003-12-31 12:34:56 } { incr i -# set sqlite_current_time [execsql "SELECT strftime('%s','$date $time')"] - set sqlite_current_time [clock scan "$date $time"] + set sqlite_current_time [clock scan "$date $time" -gmt 1] + # set sqlite_current_time [execsql "SELECT strftime('%s','$date $time')"] do_test table-13.2.$i { execsql " INSERT INTO tablet8(a) VALUES($i); @@ -557,4 +560,72 @@ foreach {date time} { } set sqlite_current_time 0 +#-------------------------------------------------------------------- +# Test cases table-14.* +# +# Test that a table cannot be created or dropped while other virtual +# machines are active. This is required because otherwise when in +# auto-vacuum mode the btree-layer may need to move the root-pages of +# a table for which there is an open cursor. +# + +# Try to create a table from within a callback: +do_test table-14.1 { + set rc [ + catch { + db eval {SELECT * FROM tablet8 LIMIT 1} {} { + db eval {CREATE TABLE t9(a, b, c)} + } + } msg + ] + set result [list $rc $msg] +} {1 {database table is locked}} + +do_test table-14.2 { + execsql { + CREATE TABLE t9(a, b, c) + } +} {} + +# Try to drop a table from within a callback: +do_test table-14.3 { + set rc [ + catch { + db eval {SELECT * FROM tablet8 LIMIT 1} {} { + db eval {DROP TABLE t9;} + } + } msg + ] + set result [list $rc $msg] +} {1 {database table is locked}} + +# Now attach a database and ensure that a table can be created in the +# attached database whilst in a callback from a query on the main database. +do_test table-14.4 { + file delete -force test2.db + file delete -force test2.db-journal + execsql { + attach 'test2.db' as aux; + } + db eval {SELECT * FROM tablet8 LIMIT 1} {} { + db eval {CREATE TABLE aux.t1(a, b, c)} + } +} {} + +# On the other hand, it should be impossible to drop a table when any VMs +# are active. This is because VerifyCookie instructions may have already +# been executed, and btree root-pages may not move after this (which a +# delete table might do). +do_test table-14.4 { + set rc [ + catch { + db eval {SELECT * FROM tablet8 LIMIT 1} {} { + db eval {DROP TABLE aux.t1;} + } + } msg + ] + set result [list $rc $msg] +} {1 {database table is locked}} + finish_test +