From: dan Date: Wed, 23 Sep 2009 17:29:59 +0000 (+0000) Subject: Do not check immediate foreign key constraints until the end of the statement. This... X-Git-Tag: fts3-refactor~160 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=32b09f29c9cf97b7d43b1f8ce563cbd21c1cee59;p=thirdparty%2Fsqlite.git Do not check immediate foreign key constraints until the end of the statement. This matches the postgres behaviour. FossilOrigin-Name: 1a32149cc3c722058f4ed4c81edadeb6ce5bc9e4 --- diff --git a/manifest b/manifest index 5bfcc2d012..b091982fa7 100644 --- a/manifest +++ b/manifest @@ -1,8 +1,5 @@ ------BEGIN PGP SIGNED MESSAGE----- -Hash: SHA1 - -C Modify\sthe\s".dump"\scommand\son\sthe\sCLI\sso\sthat\sit\salways\sissues\sa\nPRAGMA\sforeign_keys=OFF\sat\sthe\stop\sof\sthe\soutput. -D 2009-09-23T15:51:36 +C Do\snot\scheck\simmediate\sforeign\skey\sconstraints\suntil\sthe\send\sof\sthe\sstatement.\sThis\smatches\sthe\spostgres\sbehaviour. +D 2009-09-23T17:30:00 F Makefile.arm-wince-mingw32ce-gcc fcd5e9cd67fe88836360bb4f9ef4cb7f8e2fb5a0 F Makefile.in 4ca3f1dd6efa2075bcb27f4dc43eef749877740d F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654 @@ -119,7 +116,7 @@ F src/date.c 657ff12ca0f1195b531561afacbb38b772d16638 F src/delete.c 15499f5d10047d38e68ce991b3f88cbddb6e0931 F src/expr.c c7f3f718bd5c392344ec8694a41c1824f30cf375 F src/fault.c dc88c821842157460750d2d61a8a8b4197d047ff -F src/fkey.c 9a0afe076ce51552ce00ab0f730213798bf75720 +F src/fkey.c 89232758416a48783adbb3b48ab447432e817370 F src/func.c e536218d193b8d326aab91120bc4c6f28aa2b606 F src/global.c 271952d199a8cc59d4ce840b3bbbfd2f30c8ba32 F src/hash.c ebcaa921ffd9d86f7ea5ae16a0a29d1c871130a7 @@ -209,11 +206,11 @@ F src/update.c 4fac66ecaea13c9c13e7d3de8da66aae3cce90e2 F src/utf.c 99cf927eabb104621ba889ac0dd075fc1657ad30 F src/util.c 59d4e9456bf1fe581f415a783fa0cee6115c8f35 F src/vacuum.c 869d08eaab64e2a4eaf4ef9ea34b851892b65a75 -F src/vdbe.c 4946e2ac1124b0cb0ee882ed7628d2d8b339b8ef +F src/vdbe.c a5da14fe8d89f9ad2cd4911a9d7df79c74a6b84c F src/vdbe.h 7d5075e3fa4e5587a9be8d5e503857c825490cef -F src/vdbeInt.h 546ed25cad488c053819e19d09751d71d3ce3601 +F src/vdbeInt.h 03336c0364c94267de615b9cd7768e7264f324ac F src/vdbeapi.c 524d79eb17bbcbe31c37c908b8e01edc5c684a90 -F src/vdbeaux.c 2dc9af9b797a1a038dc1bbe26d400fc042a30eaf +F src/vdbeaux.c 32d77382469c20aa5a971a8794deb1eafa8d5cb6 F src/vdbeblob.c 3ba0f7ba1b3afce2d37a18e4f437992d430f0eae F src/vdbemem.c 0ff2b209fccade3ff6709286057b82ed7f6c1e70 F src/vtab.c 3e54fe39374e5feb8b174de32a90e7a21966025d @@ -333,7 +330,7 @@ F test/expr.test 9f521ae22f00e074959f72ce2e55d46b9ed23f68 F test/filectrl.test 8923a6dc7630f31c8a9dd3d3d740aa0922df7bf8 F test/filefmt.test 84e3d0fe9f12d0d2ac852465c6f8450aea0d6f43 F test/fkey1.test 01c7de578e11747e720c2d9aeef27f239853c4da -F test/fkey2.test 9f451a47695ee9c19772bd2107d9a3c788672b0f +F test/fkey2.test d1d78b106da32c00e40b7a4228f591cc880147ac F test/fkey_malloc.test a18bdb482c6a7b9a61865616a516584d17f04a04 F test/format4.test 1f0cac8ff3895e9359ed87e41aaabee982a812eb F test/fts1a.test 46090311f85da51bb33bd5ce84f7948359c6d8d7 @@ -756,14 +753,7 @@ F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e F tool/vdbe-compress.tcl d70ea6d8a19e3571d7ab8c9b75cba86d1173ff0f -P e3b73394bf9c0391e997079b160eace3589415ab -R 852fa5de88ad9a2f042ab0c36dd79c2c -U drh -Z 43fdb088f0ddc98f38c08bf5dc74ca34 ------BEGIN PGP SIGNATURE----- -Version: GnuPG v1.4.6 (GNU/Linux) - -iD8DBQFKukQLoxKgR168RlERAuoxAJ4o9BqLqdDvJT/TgBK1g1ugRts8MwCcCirL -iIV/srWIRfE5lVnX7rkbqzE= -=sPIl ------END PGP SIGNATURE----- +P 0755b9b697d32292f378a4b934ca1cf9f56225cd +R eee597b91fa7a9feb4c2dc1ff64a49cd +U dan +Z d23c3e5d8434b673463e1d08f68374eb diff --git a/manifest.uuid b/manifest.uuid index 3c49819b28..0e489fea43 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -0755b9b697d32292f378a4b934ca1cf9f56225cd \ No newline at end of file +1a32149cc3c722058f4ed4c81edadeb6ce5bc9e4 \ No newline at end of file diff --git a/src/fkey.c b/src/fkey.c index 76cfaff362..d79b7fb562 100644 --- a/src/fkey.c +++ b/src/fkey.c @@ -291,15 +291,13 @@ static void fkLookupParent( FKey *pFKey, /* Foreign key constraint */ int *aiCol, /* Map from parent key columns to child table columns */ int regData, /* Address of array containing child table row */ - int nIncr /* If deferred FK, increment counter by this */ + int nIncr /* Increment constraint counter by this */ ){ int i; /* Iterator variable */ Vdbe *v = sqlite3GetVdbe(pParse); /* Vdbe to add code to */ int iCur = pParse->nTab - 1; /* Cursor number to use */ int iOk = sqlite3VdbeMakeLabel(v); /* jump here if parent key found */ - assert( pFKey->isDeferred || nIncr==1 ); - /* Check if any of the key columns in the child table row are ** NULL. If any are, then the constraint is satisfied. No need ** to search for a matching row in the parent table. */ @@ -341,13 +339,20 @@ static void fkLookupParent( sqlite3ReleaseTempReg(pParse, regRec); } - if( pFKey->isDeferred ){ - assert( nIncr==1 || nIncr==-1 ); - sqlite3VdbeAddOp1(v, OP_DeferredCons, nIncr); - }else{ + if( !pFKey->isDeferred && !pParse->pToplevel && !pParse->isMultiWrite ){ + /* Special case: If this is an INSERT statement that will insert exactly + ** one row into the table, raise a constraint immediately instead of + ** incrementing a counter. This is necessary as the VM code is being + ** generated for will not open a statement transaction. */ + assert( nIncr==1 ); sqlite3HaltConstraint( pParse, OE_Abort, "foreign key constraint failed", P4_STATIC ); + }else{ + if( nIncr>0 && pFKey->isDeferred==0 ){ + sqlite3ParseToplevel(pParse)->mayAbort = 1; + } + sqlite3VdbeAddOp2(v, OP_FkCounter, nIncr, pFKey->isDeferred); } sqlite3VdbeResolveLabel(v, iOk); @@ -423,14 +428,16 @@ static void fkScanChildren( ** each row found. Otherwise, for deferred constraints, increment the ** deferred constraint counter by nIncr for each row selected. */ pWInfo = sqlite3WhereBegin(pParse, pSrc, pWhere, 0, 0); - if( pFKey->isDeferred && nIncr ){ - assert( nIncr==1 || nIncr==-1 ); - sqlite3VdbeAddOp1(pParse->pVdbe, OP_DeferredCons, nIncr); - }else{ - assert( nIncr==1 || nIncr==0 ); + if( nIncr==0 ){ + /* A RESTRICT Action. */ sqlite3HaltConstraint( pParse, OE_Abort, "foreign key constraint failed", P4_STATIC ); + }else{ + if( nIncr>0 && pFKey->isDeferred==0 ){ + sqlite3ParseToplevel(pParse)->mayAbort = 1; + } + sqlite3VdbeAddOp2(pParse->pVdbe, OP_FkCounter, nIncr, pFKey->isDeferred); } if( pWInfo ){ sqlite3WhereEnd(pWInfo); @@ -535,11 +542,6 @@ void sqlite3FkCheck( int iCol; int i; - /* If this is a DELETE operation and the foreign key is not deferred, - ** nothing to do. A DELETE on the child table cannot cause the FK - ** constraint to fail. */ - if( pFKey->isDeferred==0 && regNew==0 ) continue; - /* Find the parent table of this foreign key. Also find a unique index ** on the parent key columns in the parent table. If either of these ** schema items cannot be located, set an error in pParse and return @@ -571,10 +573,15 @@ void sqlite3FkCheck( sqlite3TableLock(pParse, iDb, pTo->tnum, 0, pTo->zName); pParse->nTab++; - if( regOld!=0 && pFKey->isDeferred ){ + if( regOld!=0 ){ + /* A row is being removed from the child table. Search for the parent. + ** If the parent does not exist, removing the child row resolves an + ** outstanding foreign key constraint violation. */ fkLookupParent(pParse, iDb, pTo, pIdx, pFKey, aiCol, regOld, -1); } if( regNew!=0 ){ + /* A row is being added to the child table. If a parent row cannot + ** be found, adding the child row has violated the FK constraint. */ fkLookupParent(pParse, iDb, pTo, pIdx, pFKey, aiCol, regNew, +1); } @@ -588,18 +595,11 @@ void sqlite3FkCheck( SrcList *pSrc; int *aiCol = 0; - /* For immediate constraints, skip this scan if: - ** - ** 1) this is an INSERT operation, or - ** 2) an UPDATE operation and the FK action is a trigger-action, or - ** 3) a DELETE operation and the FK action is a trigger-action. - ** - ** A "trigger-action" is one of CASCADE, SET DEFAULT or SET NULL. - */ - if( pFKey->isDeferred==0 ){ - if( regOld==0 ) continue; /* 1 */ - if( regNew!=0 && pFKey->aAction[1]>OE_Restrict ) continue; /* 2 */ - if( regNew==0 && pFKey->aAction[0]>OE_Restrict ) continue; /* 3 */ + if( !pFKey->isDeferred && !pParse->pToplevel && !pParse->isMultiWrite ){ + assert( regOld==0 && regNew!=0 ); + /* Inserting a single row into a parent table cannot cause an immediate + ** foreign key violation. So do nothing in this case. */ + return; } if( locateFkeyIndex(pParse, pTab, pFKey, &pIdx, &aiCol) ) return; @@ -640,7 +640,7 @@ void sqlite3FkCheck( iGoto = sqlite3VdbeAddOp0(v, OP_Goto); } - if( regNew!=0 && pFKey->isDeferred ){ + if( regNew!=0 ){ fkScanChildren(pParse, pSrc, pIdx, pFKey, aiCol, regNew, -1); } if( regOld!=0 ){ @@ -682,9 +682,7 @@ u32 sqlite3FkOldmask( FKey *p; int i; for(p=pTab->pFKey; p; p=p->pNextFrom){ - if( pChanges || p->isDeferred ){ - for(i=0; inCol; i++) mask |= COLUMN_MASK(p->aCol[i].iFrom); - } + for(i=0; inCol; i++) mask |= COLUMN_MASK(p->aCol[i].iFrom); } for(p=fkRefering(pTab); p; p=p->pNextTo){ Index *pIdx = 0; @@ -713,11 +711,7 @@ int sqlite3FkRequired( ExprList *pChanges /* Non-NULL for UPDATE operations */ ){ if( pParse->db->flags&SQLITE_ForeignKeys ){ - FKey *p; - for(p=pTab->pFKey; p; p=p->pNextFrom){ - if( pChanges || p->isDeferred ) return 1; - } - if( fkRefering(pTab) ) return 1; + if( fkRefering(pTab) || pTab->pFKey ) return 1; } return 0; } diff --git a/src/vdbe.c b/src/vdbe.c index b18ddc5b54..6014589ae4 100644 --- a/src/vdbe.c +++ b/src/vdbe.c @@ -1117,6 +1117,15 @@ case OP_ResultRow: { assert( pOp->p1>0 ); assert( pOp->p1+pOp->p2<=p->nMem+1 ); + /* If this statement has violated immediate foreign key constraints, do + ** not return the number of rows modified. And do not RELEASE the statement + ** transaction. It needs to be rolled back. */ + if( SQLITE_OK!=(rc = sqlite3VdbeCheckFk(p, 0)) ){ + assert( db->flags&SQLITE_CountRows ); + assert( p->usesStmtJournal ); + break; + } + /* If the SQLITE_CountRows flag is set in sqlite3.flags mask, then ** DML statements invoke this opcode to return the number of rows ** modified to the user. This is the only way that a VM that @@ -2578,7 +2587,7 @@ case OP_Savepoint: { */ int isTransaction = pSavepoint->pNext==0 && db->isTransactionSavepoint; if( isTransaction && p1==SAVEPOINT_RELEASE ){ - if( (rc = sqlite3VdbeCheckDeferred(p))!=SQLITE_OK ){ + if( (rc = sqlite3VdbeCheckFk(p, 1))!=SQLITE_OK ){ goto vdbe_return; } db->autoCommit = 1; @@ -2674,7 +2683,7 @@ case OP_AutoCommit: { assert( desiredAutoCommit==1 ); sqlite3RollbackAll(db); db->autoCommit = 1; - }else if( (rc = sqlite3VdbeCheckDeferred(p))!=SQLITE_OK ){ + }else if( (rc = sqlite3VdbeCheckFk(p, 1))!=SQLITE_OK ){ goto vdbe_return; }else{ db->autoCommit = (u8)desiredAutoCommit; @@ -4891,13 +4900,19 @@ case OP_Param: { /* out2-prerelease */ #endif /* #ifndef SQLITE_OMIT_TRIGGER */ #ifndef SQLITE_OMIT_FOREIGN_KEY -/* Opcode: DeferredCons P1 * * * * +/* Opcode: FkCounter P1 P2 * * * ** -** Increment the database handles "deferred constraint violation" counter -** by P1 (P1 may be negative or positive). +** Increment a "constraint counter" by by P1 (P1 may be negative or positive). +** If P2 is non-zero, the database constraint counter is incremented +** (deferred foreign key constraints). Otherwise, if P2 is zero, the +** statement counter is incremented (immediate foreign key constraints). */ -case OP_DeferredCons: { - db->nDeferredCons += pOp->p1; +case OP_FkCounter: { + if( pOp->p2 ){ + db->nDeferredCons += pOp->p1; + }else{ + p->nFkConstraint += pOp->p1; + } break; } #endif /* #ifndef SQLITE_OMIT_FOREIGN_KEY */ diff --git a/src/vdbeInt.h b/src/vdbeInt.h index 0c3ffadc80..b24b3fb429 100644 --- a/src/vdbeInt.h +++ b/src/vdbeInt.h @@ -315,6 +315,7 @@ struct Vdbe { int aCounter[2]; /* Counters used by sqlite3_stmt_status() */ char *zSql; /* Text of the SQL statement that generated this */ void *pFree; /* Free this when deleting the vdbe */ + i64 nFkConstraint; /* Number of imm. FK constraints this VM */ i64 nStmtDefCons; /* Number of def. constraints when stmt started */ int iStatement; /* Statement number (or 0 if has not opened stmt) */ #ifdef SQLITE_DEBUG diff --git a/src/vdbeaux.c b/src/vdbeaux.c index fdf223e3bb..b5e041b72b 100644 --- a/src/vdbeaux.c +++ b/src/vdbeaux.c @@ -321,6 +321,7 @@ static Op *opIterNext(VdbeOpIter *p){ ** * OP_Destroy ** * OP_VUpdate ** * OP_VRename +** * OP_FkCounter with P2==0 (immediate foreign key constraint) ** ** Then check that the value of Parse.mayAbort is true if an ** ABORT may be thrown, or false otherwise. Return true if it does @@ -339,6 +340,9 @@ int sqlite3VdbeAssertMayAbort(Vdbe *v, int mayAbort){ while( (pOp = opIterNext(&sIter))!=0 ){ int opcode = pOp->opcode; if( opcode==OP_Destroy || opcode==OP_VUpdate || opcode==OP_VRename +#ifndef SQLITE_OMIT_FOREIGN_KEY + || (opcode==OP_FkCounter && pOp->p1==1 && pOp->p2==0) +#endif || ((opcode==OP_Halt || opcode==OP_HaltIfNull) && (pOp->p1==SQLITE_CONSTRAINT && pOp->p2==OE_Abort)) ){ @@ -1944,10 +1948,11 @@ void sqlite3VdbeMutexArrayEnter(Vdbe *p){ ** an error message to it. Then return SQLITE_ERROR. */ #ifndef SQLITE_OMIT_FOREIGN_KEY -int sqlite3VdbeCheckDeferred(Vdbe *p){ +int sqlite3VdbeCheckFk(Vdbe *p, int deferred){ sqlite3 *db = p->db; - if( db->nDeferredCons ){ + if( (deferred && db->nDeferredCons>0) || (!deferred && p->nFkConstraint>0) ){ p->rc = SQLITE_CONSTRAINT; + p->errorAction = OE_Abort; sqlite3SetString(&p->zErrMsg, db, "foreign key constraint failed"); return SQLITE_ERROR; } @@ -2029,6 +2034,11 @@ int sqlite3VdbeHalt(Vdbe *p){ } } } + + /* Check for immediate foreign key violations. */ + if( p->rc==SQLITE_OK ){ + sqlite3VdbeCheckFk(p, 0); + } /* If the auto-commit flag is set and this is the only active writer ** VM, then we do either a commit or rollback of the current transaction. @@ -2041,7 +2051,7 @@ int sqlite3VdbeHalt(Vdbe *p){ && db->writeVdbeCnt==(p->readOnly==0) ){ if( p->rc==SQLITE_OK || (p->errorAction==OE_Fail && !isSpecialError) ){ - if( sqlite3VdbeCheckDeferred(p) ){ + if( sqlite3VdbeCheckFk(p, 1) ){ sqlite3BtreeMutexArrayLeave(&p->aMutex); return SQLITE_ERROR; } diff --git a/test/fkey2.test b/test/fkey2.test index 7c7610050c..97333dcd62 100644 --- a/test/fkey2.test +++ b/test/fkey2.test @@ -70,7 +70,6 @@ proc drop_all_tables {{db db}} { } } - execsql { PRAGMA foreign_keys = on } set FkeySimpleSchema { @@ -131,14 +130,24 @@ foreach {tn zSql res} $FkeySimpleTests { drop_all_tables do_test fkey2-1.2.0 { - execsql [string map {/D/ {DEFERRABLE INITIALLY DEFERRED}} $FkeySimpleSchema - ] + execsql [string map {/D/ {DEFERRABLE INITIALLY DEFERRED}} $FkeySimpleSchema] } {} foreach {tn zSql res} $FkeySimpleTests { do_test fkey2-1.2.$tn { catchsql $zSql } $res } drop_all_tables +do_test fkey2-1.3.0 { + execsql [string map {/D/ {}} $FkeySimpleSchema] + execsql { PRAGMA count_changes = 1 } +} {} +foreach {tn zSql res} $FkeySimpleTests { + if {$res == "0 {}"} { set res {0 1} } + do_test fkey2-1.3.$tn { catchsql $zSql } $res +} +execsql { PRAGMA count_changes = 0 } +drop_all_tables + #------------------------------------------------------------------------- # This section (test cases fkey2-2.*) contains tests to check that the # deferred foreign key constraint logic works. @@ -744,6 +753,7 @@ do_test fkey2-genfkey.2.4 { SELECT * FROM t2; } } {2 one} + do_test fkey2-genfkey.2.5 { execsql { INSERT INTO t3 VALUES('hello', 2, 3);