From: dan Date: Fri, 27 Nov 2009 12:12:34 +0000 (+0000) Subject: Move [7d30880114] to the trunk. Add optimizations to reduce the number of opcodes... X-Git-Tag: version-3.7.2~789 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=bb5f168f2e76930161bd7e77a771de736b227c97;p=thirdparty%2Fsqlite.git Move [7d30880114] to the trunk. Add optimizations to reduce the number of opcodes used for BEFORE UPDATE triggers. FossilOrigin-Name: 1b7c5250ccb63182324bfc3f1ea28f17b6db357a --- diff --git a/manifest b/manifest index 8f175f44e4..22191b85d1 100644 --- a/manifest +++ b/manifest @@ -1,8 +1,5 @@ ------BEGIN PGP SIGNED MESSAGE----- -Hash: SHA1 - -C Simplifications\sto\sthe\ssqlite3_trace()\sbound\sparameter\ssubstitution\slogic. -D 2009-11-26T14:01:53 +C Move\s[7d30880114]\sto\sthe\strunk.\sAdd\soptimizations\sto\sreduce\sthe\snumber\sof\sopcodes\sused\sfor\sBEFORE\sUPDATE\striggers. +D 2009-11-27T12:12:34 F Makefile.arm-wince-mingw32ce-gcc fcd5e9cd67fe88836360bb4f9ef4cb7f8e2fb5a0 F Makefile.in c5827ead754ab32b9585487177c93bb00b9497b3 F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654 @@ -118,7 +115,7 @@ F src/build.c a48e74d24897100017d39ceba5de255e53ec9488 F src/callback.c 908f3e0172c3d4058f4ca0acd42c637c52e9669f F src/complete.c 417df1ef5ea798532bb6290b0cc4265fef82980a F src/date.c a79c0a8f219370b972e320741f995a3bef9df33f -F src/delete.c ec04635d152debdab70d4b30c5516b59282075ea +F src/delete.c 8b8afb9cd7783d573eae55a3f4208bc0637a2bb8 F src/expr.c 50385ed51f1cd7f1ab289629cd0f87d5b2fcca52 F src/fault.c 160a0c015b6c2629d3899ed2daf63d75754a32bb F src/fkey.c e2116672a6bd610dc888e27df292ebc7999c9bb0 @@ -163,13 +160,13 @@ F src/pragma.c 6936d7df5e04b9f996f8f320d15e65b6944b2caa F src/prepare.c ad90970bba3aead154266d8bb6faf9fbb5233b94 F src/printf.c 644bc7d59df3dc56d6d8b9a510914bfc6b51bc69 F src/random.c cd4a67b3953b88019f8cd4ccd81394a8ddfaba50 -F src/resolve.c c52d9e52e11058f4113f6644adc20d3f85141b1d +F src/resolve.c d052e5c44bab34f83b3c1741aaa07478d18b5dd5 F src/rowset.c 69afa95a97c524ba6faf3805e717b5b7ae85a697 F src/select.c 2f9ed7482e7a25b0b127fc41693bbdbe1caf5647 F src/shell.c f4948cb6d30665d755a6b5e0ec313d1094aab828 F src/sqlite.h.in 4464e9772122f0447305d425e04d122b6f1bffec F src/sqlite3ext.h 69dfb8116af51b84a029cddb3b35062354270c89 -F src/sqliteInt.h f79536c70bba65124b6fda0b09dfc35f1f513f51 +F src/sqliteInt.h f09be5c67f95f3d28d44e5b608b18cab28758ba4 F src/sqliteLimit.h 3afab2291762b5d09ae20c18feb8e9fa935a60a6 F src/status.c e651be6b30d397d86384c6867bc016e4913bcac7 F src/table.c 2cd62736f845d82200acfa1287e33feb3c15d62e @@ -207,8 +204,8 @@ F src/test_tclvar.c f4dc67d5f780707210d6bb0eb6016a431c04c7fa F src/test_thread.c 00fed80690ae7f1525483a35861511c48bc579f2 F src/test_wsd.c 41cadfd9d97fe8e3e4e44f61a4a8ccd6f7ca8fe9 F src/tokenize.c 9f7d39da1a1346fa0cf106ea0bf10bb6b8b61ddf -F src/trigger.c 3c48db13db94f3c34b01b7caae2130279c8f4d28 -F src/update.c 8efeb09822886e33c265dd96d29a3d865ea6dcf2 +F src/trigger.c d46f9389e3bf3dd1cc1d288aba2f289c96b34200 +F src/update.c edf5649ffc9fca6d6884e34b6da6358e724919fd F src/utf.c dad16adcc0c35ef2437dca125a4b07419d361052 F src/util.c ad4f03079ba0fe83590d1cc9197e8e4844e38592 F src/vacuum.c 03309a08d549f9389cc3a3589afd4fadbdaf0679 @@ -706,7 +703,7 @@ F test/trigger8.test 30cb0530bd7c4728055420e3f739aa00412eafa4 F test/trigger9.test 5b0789f1c5c4600961f8e68511b825b87be53e31 F test/triggerA.test 0718ad2d9bfef27c7af00e636df79bee6b988da7 F test/triggerB.test 56780c031b454abac2340dbb3b71ac5c56c3d7fe -F test/triggerC.test 0acc1d22d9c3d6cf0018bf795d544732a25657dc +F test/triggerC.test 4083c64d80854d271bad211268a08985f3d61cbd F test/types.test 9a825ec8eea4e965d7113b74c76a78bb5240f2ac F test/types2.test 3555aacf8ed8dc883356e59efc314707e6247a84 F test/types3.test a0f66bf12f80fad89493535474f7a6d16fa58150 @@ -777,14 +774,7 @@ F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e F tool/vdbe-compress.tcl d70ea6d8a19e3571d7ab8c9b75cba86d1173ff0f -P f25558f333637b83f98a649acbb8a0d5dbada9ba -R 7297b93801cb1b5f0f2d094aa4774976 -U drh -Z 9cc025af73edc8bbc9ede953398b7222 ------BEGIN PGP SIGNATURE----- -Version: GnuPG v1.4.6 (GNU/Linux) - -iD8DBQFLDopUoxKgR168RlERAiBYAJ9naKwYidFTksJBSSLPAu1eSYKX2QCfe441 -vCSQUiBaqpFdB03sRokfcyc= -=Cbq4 ------END PGP SIGNATURE----- +P cb4b928648504ce29d751834e9ee3b5278dfca65 +R 38ca3dab21e3d85a61a2ba8f76e8fc34 +U dan +Z 0eaf554ff6bd1729f2cb11e15acc51b6 diff --git a/manifest.uuid b/manifest.uuid index d96d7e81a5..152bb9b6cd 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -cb4b928648504ce29d751834e9ee3b5278dfca65 \ No newline at end of file +1b7c5250ccb63182324bfc3f1ea28f17b6db357a \ No newline at end of file diff --git a/src/delete.c b/src/delete.c index 367b13d5b6..5b30888280 100644 --- a/src/delete.c +++ b/src/delete.c @@ -496,7 +496,9 @@ void sqlite3GenerateRowDelete( /* TODO: Could use temporary registers here. Also could attempt to ** avoid copying the contents of the rowid register. */ - mask = sqlite3TriggerOldmask(pParse, pTrigger, 0, pTab, onconf); + mask = sqlite3TriggerColmask( + pParse, pTrigger, 0, 0, TRIGGER_BEFORE|TRIGGER_AFTER, pTab, onconf + ); mask |= sqlite3FkOldmask(pParse, pTab); iOld = pParse->nMem+1; pParse->nMem += (1 + pTab->nCol); diff --git a/src/resolve.c b/src/resolve.c index d913a24c8a..0a59be7c44 100644 --- a/src/resolve.c +++ b/src/resolve.c @@ -264,6 +264,10 @@ static int lookupName( testcase( iCol==31 ); testcase( iCol==32 ); pParse->oldmask |= (iCol>=32 ? 0xffffffff : (((u32)1)<newmask |= (iCol>=32 ? 0xffffffff : (((u32)1)<iColumn = (i16)iCol; pExpr->pTab = pTab; diff --git a/src/sqliteInt.h b/src/sqliteInt.h index 334a39a0f7..aae1c3c663 100644 --- a/src/sqliteInt.h +++ b/src/sqliteInt.h @@ -2059,15 +2059,16 @@ struct AutoincInfo { ** The Parse.pTriggerPrg list never contains two entries with the same ** values for both pTrigger and orconf. ** -** The TriggerPrg.oldmask variable is set to a mask of old.* columns +** The TriggerPrg.aColmask[0] variable is set to a mask of old.* columns ** accessed (or set to 0 for triggers fired as a result of INSERT -** statements). +** statements). Similarly, the TriggerPrg.aColmask[1] variable is set to +** a mask of new.* columns used by the program. */ struct TriggerPrg { Trigger *pTrigger; /* Trigger this program was coded from */ int orconf; /* Default ON CONFLICT policy */ SubProgram *pProgram; /* Program implementing pTrigger/orconf */ - u32 oldmask; /* Mask of old.* columns accessed */ + u32 aColmask[2]; /* Masks of old.*, new.* columns accessed */ TriggerPrg *pNext; /* Next entry in Parse.pTriggerPrg list */ }; @@ -2139,6 +2140,7 @@ struct Parse { Parse *pToplevel; /* Parse structure for main program (or NULL) */ Table *pTriggerTab; /* Table triggers are being coded for */ u32 oldmask; /* Mask of old.* columns referenced */ + u32 newmask; /* Mask of new.* columns referenced */ u8 eTriggerOp; /* TK_UPDATE, TK_INSERT or TK_DELETE */ u8 eOrconf; /* Default ON CONFLICT policy for trigger steps */ u8 disableTriggers; /* True to disable triggers */ @@ -2736,7 +2738,7 @@ void sqlite3MaterializeView(Parse*, Table*, Expr*, int); TriggerStep *sqlite3TriggerDeleteStep(sqlite3*,Token*, Expr*); void sqlite3DeleteTrigger(sqlite3*, Trigger*); void sqlite3UnlinkAndDeleteTrigger(sqlite3*,int,const char*); - u32 sqlite3TriggerOldmask(Parse*,Trigger*,ExprList*,Table*,int); + u32 sqlite3TriggerColmask(Parse*,Trigger*,ExprList*,int,int,Table*,int); # define sqlite3ParseToplevel(p) ((p)->pToplevel ? (p)->pToplevel : (p)) #else # define sqlite3TriggersExist(B,C,D,E,F) 0 @@ -2747,7 +2749,7 @@ void sqlite3MaterializeView(Parse*, Table*, Expr*, int); # define sqlite3CodeRowTriggerDirect(A,B,C,D,E,F) # define sqlite3TriggerList(X, Y) 0 # define sqlite3ParseToplevel(p) p -# define sqlite3TriggerOldmask(A,B,C,D,E) 0 +# define sqlite3TriggerColmask(A,B,C,D,E,F,G) 0 #endif int sqlite3JoinType(Parse*, Token*, Token*, Token*); diff --git a/src/trigger.c b/src/trigger.c index b1256190c1..51969ce230 100644 --- a/src/trigger.c +++ b/src/trigger.c @@ -811,7 +811,8 @@ static TriggerPrg *codeRowTrigger( pProgram->nRef = 1; pPrg->pTrigger = pTrigger; pPrg->orconf = orconf; - pPrg->oldmask = 0xffffffff; + pPrg->aColmask[0] = 0xffffffff; + pPrg->aColmask[1] = 0xffffffff; /* Allocate and populate a new Parse context to use for coding the ** trigger sub-program. */ @@ -872,7 +873,8 @@ static TriggerPrg *codeRowTrigger( pProgram->nMem = pSubParse->nMem; pProgram->nCsr = pSubParse->nTab; pProgram->token = (void *)pTrigger; - pPrg->oldmask = pSubParse->oldmask; + pPrg->aColmask[0] = pSubParse->oldmask; + pPrg->aColmask[1] = pSubParse->newmask; sqlite3VdbeDelete(v); } @@ -1032,28 +1034,36 @@ void sqlite3CodeRowTrigger( } /* -** Triggers fired by UPDATE or DELETE statements may access values stored -** in the old.* pseudo-table. This function returns a 32-bit bitmask -** indicating which columns of the old.* table actually are used by -** triggers. This information may be used by the caller to avoid having -** to load the entire old.* record into memory when executing an UPDATE -** or DELETE command. +** Triggers may access values stored in the old.* or new.* pseudo-table. +** This function returns a 32-bit bitmask indicating which columns of the +** old.* or new.* tables actually are used by triggers. This information +** may be used by the caller, for example, to avoid having to load the entire +** old.* record into memory when executing an UPDATE or DELETE command. ** ** Bit 0 of the returned mask is set if the left-most column of the -** table may be accessed using an old. reference. Bit 1 is set if +** table may be accessed using an [old|new]. reference. Bit 1 is set if ** the second leftmost column value is required, and so on. If there ** are more than 32 columns in the table, and at least one of the columns ** with an index greater than 32 may be accessed, 0xffffffff is returned. ** -** It is not possible to determine if the old.rowid column is accessed -** by triggers. The caller must always assume that it is. +** It is not possible to determine if the old.rowid or new.rowid column is +** accessed by triggers. The caller must always assume that it is. ** -** There is no equivalent function for new.* references. +** Parameter isNew must be either 1 or 0. If it is 0, then the mask returned +** applies to the old.* table. If 1, the new.* table. +** +** Parameter tr_tm must be a mask with one or both of the TRIGGER_BEFORE +** and TRIGGER_AFTER bits set. Values accessed by BEFORE triggers are only +** included in the returned mask if the TRIGGER_BEFORE bit is set in the +** tr_tm parameter. Similarly, values accessed by AFTER triggers are only +** included in the returned mask if the TRIGGER_AFTER bit is set in tr_tm. */ -u32 sqlite3TriggerOldmask( +u32 sqlite3TriggerColmask( Parse *pParse, /* Parse context */ Trigger *pTrigger, /* List of triggers on table pTab */ ExprList *pChanges, /* Changes list for any UPDATE OF triggers */ + int isNew, /* 1 for new.* ref mask, 0 for old.* ref mask */ + int tr_tm, /* Mask of TRIGGER_BEFORE|TRIGGER_AFTER */ Table *pTab, /* The table to code triggers from */ int orconf /* Default ON CONFLICT policy for trigger steps */ ){ @@ -1061,12 +1071,15 @@ u32 sqlite3TriggerOldmask( u32 mask = 0; Trigger *p; + assert( isNew==1 || isNew==0 ); for(p=pTrigger; p; p=p->pNext){ - if( p->op==op && checkColumnOverlap(p->pColumns,pChanges) ){ + if( p->op==op && (tr_tm&p->tr_tm) + && checkColumnOverlap(p->pColumns,pChanges) + ){ TriggerPrg *pPrg; pPrg = getRowTrigger(pParse, p, pTab, orconf); if( pPrg ){ - mask |= pPrg->oldmask; + mask |= pPrg->aColmask[isNew]; } } } diff --git a/src/update.c b/src/update.c index c23601592d..c927059aa5 100644 --- a/src/update.c +++ b/src/update.c @@ -111,14 +111,15 @@ void sqlite3Update( AuthContext sContext; /* The authorization context */ NameContext sNC; /* The name-context to resolve expressions in */ int iDb; /* Database containing the table being updated */ - int j1; /* Addresses of jump instructions */ int okOnePass; /* True for one-pass algorithm without the FIFO */ int hasFK; /* True if foreign key processing is required */ #ifndef SQLITE_OMIT_TRIGGER - int isView; /* Trying to update a view */ - Trigger *pTrigger; /* List of triggers on pTab, if required */ + int isView; /* True when updating a view (INSTEAD OF trigger) */ + Trigger *pTrigger; /* List of triggers on pTab, if required */ + int tmask; /* Mask of TRIGGER_BEFORE|TRIGGER_AFTER */ #endif + int newmask; /* Register Allocations */ int regRowCount = 0; /* A count of rows changed */ @@ -146,11 +147,13 @@ void sqlite3Update( ** updated is a view. */ #ifndef SQLITE_OMIT_TRIGGER - pTrigger = sqlite3TriggersExist(pParse, pTab, TK_UPDATE, pChanges, 0); + pTrigger = sqlite3TriggersExist(pParse, pTab, TK_UPDATE, pChanges, &tmask); isView = pTab->pSelect!=0; + assert( pTrigger || tmask==0 ); #else # define pTrigger 0 # define isView 0 +# define tmask 0 #endif #ifdef SQLITE_OMIT_VIEW # undef isView @@ -160,7 +163,7 @@ void sqlite3Update( if( sqlite3ViewGetColumnNames(pParse, pTab) ){ goto update_cleanup; } - if( sqlite3IsReadOnly(pParse, pTab, (pTrigger?1:0)) ){ + if( sqlite3IsReadOnly(pParse, pTab, tmask) ){ goto update_cleanup; } aXRef = sqlite3DbMallocRaw(db, sizeof(int) * pTab->nCol ); @@ -388,7 +391,9 @@ void sqlite3Update( ** with the required old.* column data. */ if( hasFK || pTrigger ){ u32 oldmask = (hasFK ? sqlite3FkOldmask(pParse, pTab) : 0); - oldmask |= sqlite3TriggerOldmask(pParse, pTrigger, pChanges, pTab, onError); + oldmask |= sqlite3TriggerColmask(pParse, + pTrigger, pChanges, 0, TRIGGER_BEFORE|TRIGGER_AFTER, pTab, onError + ); for(i=0; inCol; i++){ if( aXRef[i]<0 || oldmask==0xffffffff || (oldmask & (1<nCol; i++){ if( i==pTab->iPKey ){ sqlite3VdbeAddOp2(v, OP_Null, 0, regNew+i); }else{ j = aXRef[i]; - if( j<0 ){ + if( j>=0 ){ + sqlite3ExprCode(pParse, pChanges->a[j].pExpr, regNew+i); + }else if( 0==(tmask&TRIGGER_BEFORE) || i>31 || (newmask&(1<a[j].pExpr, regNew+i); } } } /* Fire any BEFORE UPDATE triggers. This happens before constraints are - ** verified. One could argue that this is wrong. */ - if( pTrigger ){ + ** verified. One could argue that this is wrong. + */ + if( tmask&TRIGGER_BEFORE ){ sqlite3VdbeAddOp2(v, OP_Affinity, regNew, pTab->nCol); sqlite3TableAffinityStr(v, pTab); sqlite3CodeRowTrigger(pParse, pTrigger, TK_UPDATE, pChanges, @@ -432,11 +455,25 @@ void sqlite3Update( ** case, jump to the next row. No updates or AFTER triggers are ** required. This behaviour - what happens when the row being updated ** is deleted or renamed by a BEFORE trigger - is left undefined in the - ** documentation. */ + ** documentation. + */ sqlite3VdbeAddOp3(v, OP_NotExists, iCur, addr, regOldRowid); + + /* If it did not delete it, the row-trigger may still have modified + ** some of the columns of the row being updated. Load the values for + ** all columns not modified by the update statement into their + ** registers in case this has happened. + */ + for(i=0; inCol; i++){ + if( aXRef[i]<0 && i!=pTab->iPKey ){ + sqlite3VdbeAddOp3(v, OP_Column, iCur, i, regNew+i); + sqlite3ColumnDefault(v, pTab, i, regNew+i); + } + } } if( !isView ){ + int j1; /* Address of jump instruction */ /* Do constraint checks. */ sqlite3GenerateConstraintChecks(pParse, pTab, iCur, regNewRowid, diff --git a/test/triggerC.test b/test/triggerC.test index a86269f1d4..c1967bedb6 100644 --- a/test/triggerC.test +++ b/test/triggerC.test @@ -784,8 +784,76 @@ do_test triggerC-9.2 { SELECT a FROM t9 ORDER BY a; } } {1 2 3 4} - +# At one point (between versions 3.6.18 and 3.6.20 inclusive), an UPDATE +# that fired a BEFORE trigger that itself updated the same row as the +# statement causing it to fire was causing a strange side-effect: The +# values updated by the statement within the trigger were being overwritten +# by the values in the new.* array, even if those values were not +# themselves written by the parent UPDATE statement. +# +# Technically speaking this was not a bug. The SQLite documentation says +# that if a BEFORE UPDATE or BEFORE DELETE trigger modifies or deletes the +# row that the parent statement is operating on the results are undefined. +# But as of 3.6.21 behaviour is restored to the way it was in versions +# 3.6.17 and earlier to avoid causing unnecessary difficulties. +# +do_test triggerC-10.1 { + execsql { + CREATE TABLE t10(a, updatecnt DEFAULT 0); + CREATE TRIGGER t10_bu BEFORE UPDATE OF a ON t10 BEGIN + UPDATE t10 SET updatecnt = updatecnt+1 WHERE rowid = old.rowid; + END; + INSERT INTO t10(a) VALUES('hello'); + } + + # Before the problem was fixed, table t10 would contain the tuple + # (world, 0) after running the following script (because the value + # 1 written to column "updatecnt" was clobbered by the old value 0). + # + execsql { + UPDATE t10 SET a = 'world'; + SELECT * FROM t10; + } +} {world 1} + +do_test triggerC-10.2 { + execsql { + UPDATE t10 SET a = 'tcl', updatecnt = 5; + SELECT * FROM t10; + } +} {tcl 5} + +do_test triggerC-10.3 { + execsql { + CREATE TABLE t11( + c1, c2, c3, c4, c5, c6, c7, c8, c9, c10, + c11, c12, c13, c14, c15, c16, c17, c18, c19, c20, + c21, c22, c23, c24, c25, c26, c27, c28, c29, c30, + c31, c32, c33, c34, c35, c36, c37, c38, c39, c40 + ); + + CREATE TRIGGER t11_bu BEFORE UPDATE OF c1 ON t11 BEGIN + UPDATE t11 SET c31 = c31+1, c32=c32+1 WHERE rowid = old.rowid; + END; + + INSERT INTO t11 VALUES( + 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, + 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, + 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, + 31, 32, 33, 34, 35, 36, 37, 38, 39, 40 + ); + } + + # Before the problem was fixed, table t10 would contain the tuple + # (world, 0) after running the following script (because the value + # 1 written to column "updatecnt" was clobbered by the old value 0). + # + execsql { + UPDATE t11 SET c4=35, c33=22, c1=5; + SELECT * FROM t11; + } +} {5 2 3 35 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 32 33 22 34 35 36 37 38 39 40} finish_test