From 7f7512214963e538b12bc37e0dcef85732c7d42e Mon Sep 17 00:00:00 2001 From: drh Date: Tue, 17 Mar 2009 22:33:00 +0000 Subject: [PATCH] Move the rowid cache out of VdbeCursor and into BtCursor. When multiple BtCursors are open on the same table, set their rowid cache all at the same time. Ticket #3731. (CVS 6354) FossilOrigin-Name: 189785832a7dc9f4a0a2113d850b92b987e0f9bf --- manifest | 23 +++++++++++---------- manifest.uuid | 2 +- src/btree.c | 52 ++++++++++++++++++++++++++++++++++++++++++++--- src/btree.h | 4 +++- src/btreeInt.h | 3 ++- src/vdbe.c | 20 ++++++------------ src/vdbeInt.h | 4 +--- test/tkt3731.test | 51 ++++++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 125 insertions(+), 34 deletions(-) create mode 100644 test/tkt3731.test diff --git a/manifest b/manifest index bbf3ef2a65..e89d0e548f 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Fix\sa\sproblem\sthat\scould\scause\sa\scrash\swhen\sa\sshared-cache\sschema\scontains\scolumn\sdefault\svalues.\s(CVS\s6353) -D 2009-03-17T17:49:00 +C Move\sthe\srowid\scache\sout\sof\sVdbeCursor\sand\sinto\sBtCursor.\s\sWhen\smultiple\nBtCursors\sare\sopen\son\sthe\ssame\stable,\sset\stheir\srowid\scache\sall\sat\sthe\nsame\stime.\s\sTicket\s#3731.\s(CVS\s6354) +D 2009-03-17T22:33:01 F Makefile.arm-wince-mingw32ce-gcc fcd5e9cd67fe88836360bb4f9ef4cb7f8e2fb5a0 F Makefile.in 583e87706abc3026960ed759aff6371faf84c211 F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654 @@ -103,9 +103,9 @@ F src/auth.c c8b2ab5c8bad4bd90ed7c294694f48269162c627 F src/backup.c 0082d0e5a63f04e88faee0dff0a7d63d3e92a78d F src/bitvec.c 44f7059ac1f874d364b34af31b9617e52223ba75 F src/btmutex.c 341502bc496dc0840dcb00cde65680fb0e85c3ab -F src/btree.c 7d1c7e15ef8f5b139a9267c3f053d10f8885759b -F src/btree.h 96a019c9f28da38e79940512d7800e419cd8c702 -F src/btreeInt.h 2f1fe3c5f48800de8b542e05a746796cfdda13bd +F src/btree.c 805ca9606405ae80fe1afa001e7054f13ecb6046 +F src/btree.h 8ed34391fe4288c48bc540d80103f3ffabf3d3a5 +F src/btreeInt.h 6d0c6fad247f27a09af1bb4f810bfad232aefaa7 F src/build.c 5f050f06ee4219689e211fa47fd3cc8a817ede57 F src/callback.c 09c6fedc77a45db99ba25a75d61382830314b357 F src/complete.c cb14e06dbe79dee031031f0d9e686ff306afe07c @@ -200,9 +200,9 @@ F src/update.c 8ededddcde6f7b6da981dd0429a5d34518a475b7 F src/utf.c 1da9c832dba0fa8f865b5b902d93f420a1ee4245 F src/util.c 469d74f5bf09ed6398702c7da2ef8a34e979a1c1 F src/vacuum.c 4929a585ef0fb1dfaf46302f8a9c4aa30c2d9cf5 -F src/vdbe.c 230b0b7e73d90e001e811fca94b95b664b6ece66 +F src/vdbe.c 2ae831d43cb22b42dbbe90555bd57e053af53469 F src/vdbe.h d70a68bee196ab228914a3902c79dbd24342a0f2 -F src/vdbeInt.h d12bc259b34d3d610ebf05d648eb6346d48478c3 +F src/vdbeInt.h 8732228974ca23aedfd74c37a444b7bede857927 F src/vdbeapi.c ffd5d8b493590da6e09fd54b1bea1a9d38247f11 F src/vdbeaux.c feeafee5f9de51c0d30907e0600ce4db5d032df8 F src/vdbeblob.c 2852bae14c87129835938db58a77c3121e3ae962 @@ -628,6 +628,7 @@ F test/tkt3554.test 0463fea3650ac18d3694a8f39e6e25cddc1ac1fc F test/tkt3581.test 1966b7193f1e3f14951cce8c66907ae69454e9a3 F test/tkt35xx.test 53bca895091e968126a858ee7da186f59f328994 F test/tkt3630.test 929f64852103054125200bc825c316d5f75d42f7 +F test/tkt3731.test 8a6e3732f5a8a24eb875a6faf287ef77bb8c0579 F test/tokenize.test ce430a7aed48fc98301611429595883fdfcab5d7 F test/trace.test 951cd0f5f571e7f36bf7bfe04be70f90fb16fb00 F test/trans.test b3f0c696ddf8c3f113fd2edf49318b2bf431c99a @@ -707,7 +708,7 @@ F tool/speedtest16.c c8a9c793df96db7e4933f0852abb7a03d48f2e81 F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e -P d137e841ce90be92f4c18218f35d22e3a06b0f14 -R 6b134cd3537c64ea60668297de172f94 -U danielk1977 -Z fe79ce498897fec0bf8c493bbec253f1 +P afadddc34eee3d6a39102b790ce1a869b33d4286 +R a87ad391e70921cc51f370f982e9d47a +U drh +Z 4d6edfbd554e5429583cecea6d7e5900 diff --git a/manifest.uuid b/manifest.uuid index 499a81405c..643d99ae91 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -afadddc34eee3d6a39102b790ce1a869b33d4286 \ No newline at end of file +189785832a7dc9f4a0a2113d850b92b987e0f9bf \ No newline at end of file diff --git a/src/btree.c b/src/btree.c index ed12137b82..15fe1e8a3d 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.573 2009/03/16 13:19:36 danielk1977 Exp $ +** $Id: btree.c,v 1.574 2009/03/17 22:33:01 drh Exp $ ** ** This file implements a external (disk-based) database using BTrees. ** See the header comment on "btreeInt.h" for additional information. @@ -1764,6 +1764,12 @@ int sqlite3BtreeSetPageSize(Btree *p, int pageSize, int nReserve){ int sqlite3BtreeGetPageSize(Btree *p){ return p->pBt->pageSize; } + +/* +** Return the number of bytes of space at the end of every page that +** are intentually left unused. This is the "reserved" space that is +** sometimes used by extensions. +*/ int sqlite3BtreeGetReserve(Btree *p){ int n; sqlite3BtreeEnter(p); @@ -3005,6 +3011,7 @@ static int btreeCursor( } pBt->pCursor = pCur; pCur->eState = CURSOR_INVALID; + pCur->cachedRowid = 0; return SQLITE_OK; @@ -3027,11 +3034,48 @@ int sqlite3BtreeCursor( sqlite3BtreeLeave(p); return rc; } -int sqlite3BtreeCursorSize(){ + +/* +** Return the size of a BtCursor object in bytes. +** +** This interfaces is needed so that users of cursors can preallocate +** sufficient storage to hold a cursor. The BtCursor object is opaque +** to users so they cannot do the sizeof() themselves - they must call +** this routine. +*/ +int sqlite3BtreeCursorSize(void){ return sizeof(BtCursor); } +/* +** Set the cached rowid value of every cursor in the same database file +** as pCur and having the same root page number as pCur. The value is +** set to iRowid. +** +** Only positive rowid values are considered valid for this cache. +** The cache is initialized to zero, indicating an invalid cache. +** A btree will work fine with zero or negative rowids. We just cannot +** cache zero or negative rowids, which means tables that use zero or +** negative rowids might run a little slower. But in practice, zero +** or negative rowids are very uncommon so this should not be a problem. +*/ +void sqlite3BtreeSetCachedRowid(BtCursor *pCur, sqlite3_int64 iRowid){ + BtCursor *p; + for(p=pCur->pBt->pCursor; p; p=p->pNext){ + if( p->pgnoRoot==pCur->pgnoRoot ) p->cachedRowid = iRowid; + } + assert( pCur->cachedRowid==iRowid ); +} +/* +** Return the cached rowid for the given cursor. A negative or zero +** return value indicates that the rowid cache is invalid and should be +** ignored. If the rowid cache has never before been set, then a +** zero is returned. +*/ +sqlite3_int64 sqlite3BtreeGetCachedRowid(BtCursor *pCur){ + return pCur->cachedRowid; +} /* ** Close a cursor. The read lock on the database file is released @@ -3093,6 +3137,8 @@ void sqlite3BtreeReleaseTempCursor(BtCursor *pCur){ sqlite3_free(pCur->pKey); } + + /* ** Make sure the BtCursor* given in the argument has a valid ** BtCursor.info structure. If it is not already valid, call @@ -3530,7 +3576,7 @@ int sqlite3BtreeData(BtCursor *pCur, u32 offset, u32 amt, void *pBuf){ ** and data to fit on the local page and for there to be no overflow ** pages. When that is so, this routine can be used to access the ** key and data without making a copy. If the key and/or data spills -** onto overflow pages, then accessPayload() must be used to reassembly +** onto overflow pages, then accessPayload() must be used to reassemble ** the key/data and copy it into a preallocated buffer. ** ** The pointer returned by this routine looks directly into the cached diff --git a/src/btree.h b/src/btree.h index dcc2c60baa..52c5c0c517 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.109 2009/02/24 10:01:52 danielk1977 Exp $ +** @(#) $Id: btree.h,v 1.110 2009/03/17 22:33:01 drh Exp $ */ #ifndef _BTREE_H_ #define _BTREE_H_ @@ -165,6 +165,8 @@ const void *sqlite3BtreeKeyFetch(BtCursor*, int *pAmt); const void *sqlite3BtreeDataFetch(BtCursor*, int *pAmt); int sqlite3BtreeDataSize(BtCursor*, u32 *pSize); int sqlite3BtreeData(BtCursor*, u32 offset, u32 amt, void*); +void sqlite3BtreeSetCachedRowid(BtCursor*, sqlite3_int64); +sqlite3_int64 sqlite3BtreeGetCachedRowid(BtCursor*); char *sqlite3BtreeIntegrityCheck(Btree*, int *aRoot, int nRoot, int, int*); struct Pager *sqlite3BtreePager(Btree*); diff --git a/src/btreeInt.h b/src/btreeInt.h index 4652521df3..8ef5ef09db 100644 --- a/src/btreeInt.h +++ b/src/btreeInt.h @@ -9,7 +9,7 @@ ** May you share freely, never taking more than you give. ** ************************************************************************* -** $Id: btreeInt.h,v 1.43 2009/03/16 13:19:36 danielk1977 Exp $ +** $Id: btreeInt.h,v 1.44 2009/03/17 22:33:01 drh Exp $ ** ** This file implements a external (disk-based) database using BTrees. ** For a detailed discussion of BTrees, refer to @@ -458,6 +458,7 @@ struct BtCursor { BtCursor *pNext, *pPrev; /* Forms a linked list of all cursors */ struct KeyInfo *pKeyInfo; /* Argument passed to comparison function */ Pgno pgnoRoot; /* The root page of this tree */ + sqlite3_int64 cachedRowid; /* Next rowid cache. 0 means not valid */ CellInfo info; /* A parse of the cell we are pointing at */ u8 wrFlag; /* True if writable */ u8 atLast; /* Cursor pointing to the last entry */ diff --git a/src/vdbe.c b/src/vdbe.c index 26e84e32cf..d509068f24 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.825 2009/03/16 13:19:36 danielk1977 Exp $ +** $Id: vdbe.c,v 1.826 2009/03/17 22:33:01 drh Exp $ */ #include "sqliteInt.h" #include "vdbeInt.h" @@ -3550,9 +3550,8 @@ case OP_NewRowid: { /* out2-prerelease */ #endif if( !pC->useRandomRowid ){ - if( pC->nextRowidValid ){ - v = pC->nextRowid; - }else{ + v = sqlite3BtreeGetCachedRowid(pC->pCursor); + if( v==0 ){ rc = sqlite3BtreeLast(pC->pCursor, &res); if( rc!=SQLITE_OK ){ goto abort_due_to_error; @@ -3589,12 +3588,7 @@ case OP_NewRowid: { /* out2-prerelease */ } #endif - if( vnextRowidValid = 1; - pC->nextRowid = v+1; - }else{ - pC->nextRowidValid = 0; - } + sqlite3BtreeSetCachedRowid(pC->pCursor, vuseRandomRowid ){ assert( pOp->p3==0 ); /* SQLITE_FULL must have occurred prior to this */ @@ -3672,9 +3666,6 @@ case OP_Insert: { iKey = intToKey(pKey->u.i); if( pOp->p5 & OPFLAG_NCHANGE ) p->nChange++; if( pOp->p5 & OPFLAG_LASTROWID ) db->lastRowid = pKey->u.i; - if( pC->nextRowidValid && pKey->u.i>=pC->nextRowid ){ - pC->nextRowidValid = 0; - } if( pData->flags & MEM_Null ){ pData->z = 0; pData->n = 0; @@ -3709,6 +3700,7 @@ case OP_Insert: { }else{ nZero = 0; } + sqlite3BtreeSetCachedRowid(pC->pCursor, 0); rc = sqlite3BtreeInsert(pC->pCursor, 0, iKey, pData->z, pData->n, nZero, pOp->p5 & OPFLAG_APPEND); @@ -3771,8 +3763,8 @@ case OP_Delete: { rc = sqlite3VdbeCursorMoveto(pC); if( rc ) goto abort_due_to_error; + sqlite3BtreeSetCachedRowid(pC->pCursor, 0); rc = sqlite3BtreeDelete(pC->pCursor); - pC->nextRowidValid = 0; pC->cacheStatus = CACHE_STALE; /* Invoke the update-hook if required. */ diff --git a/src/vdbeInt.h b/src/vdbeInt.h index 48e6c60943..1d7ed689da 100644 --- a/src/vdbeInt.h +++ b/src/vdbeInt.h @@ -15,7 +15,7 @@ ** 6000 lines long) it was split up into several smaller files and ** this header information was factored out. ** -** $Id: vdbeInt.h,v 1.164 2009/02/20 01:28:59 drh Exp $ +** $Id: vdbeInt.h,v 1.165 2009/03/17 22:33:01 drh Exp $ */ #ifndef _VDBEINT_H_ #define _VDBEINT_H_ @@ -59,13 +59,11 @@ struct VdbeCursor { BtCursor *pCursor; /* The cursor structure of the backend */ int iDb; /* Index of cursor database in db->aDb[] (or -1) */ i64 lastRowid; /* Last rowid from a Next or NextIdx operation */ - i64 nextRowid; /* Next rowid returned by OP_NewRowid */ Bool zeroed; /* True if zeroed out and ready for reuse */ Bool rowidIsValid; /* True if lastRowid is valid */ Bool atFirst; /* True if pointing to first entry */ Bool useRandomRowid; /* Generate new record numbers semi-randomly */ Bool nullRow; /* True if pointing to a row with no data */ - Bool nextRowidValid; /* True if the nextRowid field is valid */ Bool pseudoTable; /* This is a NEW or OLD pseudo-tables of a trigger */ Bool ephemPseudoTable; Bool deferredMoveto; /* A call to sqlite3BtreeMoveto() is needed */ diff --git a/test/tkt3731.test b/test/tkt3731.test new file mode 100644 index 0000000000..0b80b464ea --- /dev/null +++ b/test/tkt3731.test @@ -0,0 +1,51 @@ +# 2009 March 18 +# +# The author disclaims copyright to this source code. In place of +# a legal notice, here is a blessing: +# +# May you do good and not evil. +# May you find forgiveness for yourself and forgive others. +# May you share freely, never taking more than you give. +# +#*********************************************************************** +# +# $Id: tkt3731.test,v 1.1 2009/03/17 22:33:01 drh Exp $ + +set testdir [file dirname $argv0] +source $testdir/tester.tcl +ifcapable {!trigger} { + finish_test + return +} + +do_test tkt3731-1.1 { + execsql { + CREATE TABLE t1(a PRIMARY KEY, b); + CREATE TRIGGER tr1 AFTER INSERT ON t1 BEGIN + INSERT INTO t1 VALUES(new.a || '+', new.b || '+'); + END; + } +} {} + +do_test tkt3731-1.2 { + execsql { + INSERT INTO t1 VALUES('a', 'b'); + INSERT INTO t1 VALUES('c', 'd'); + SELECT * FROM t1; + } +} {a b a+ b+ c d c+ d+} + +do_test tkt3731-1.3 { + execsql { + DELETE FROM t1; + CREATE TABLE t2(a, b); + INSERT INTO t2 VALUES('e', 'f'); + INSERT INTO t2 VALUES('g', 'h'); + INSERT INTO t1 SELECT * FROM t2; + SELECT * FROM t1; + } +} {e f e+ f+ g h g+ h+} + +integrity_check tkt3731-1.4 + +finish_test -- 2.47.2