From: dan Date: Mon, 28 Sep 2009 11:54:21 +0000 (+0000) Subject: Fix some foreign key constraint related problems that occur when a row refers to... X-Git-Tag: fts3-refactor~143 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9277efa3d576ab243077404bb5966f9a67a73004;p=thirdparty%2Fsqlite.git Fix some foreign key constraint related problems that occur when a row refers to itself. FossilOrigin-Name: 9e503e2d0428c9e8df878c7c6594790232cca4e0 --- diff --git a/manifest b/manifest index 741394e6dc..f7e8817713 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C When\sALTER\sTABLE\sRENAME\sTO\sis\sused\sto\schange\sthe\sname\sof\sa\stable\sthat\sis\sthe\sparent\stable\sof\sa\sforeign\skey\sconstraint,\smodify\sthat\sforeign\skey\sconstraint\sto\suse\sthe\snew\stable\sname. -D 2009-09-26T17:51:48 +C Fix\ssome\sforeign\skey\sconstraint\srelated\sproblems\sthat\soccur\swhen\sa\srow\srefers\sto\sitself. +D 2009-09-28T11:54:22 F Makefile.arm-wince-mingw32ce-gcc fcd5e9cd67fe88836360bb4f9ef4cb7f8e2fb5a0 F Makefile.in 4ca3f1dd6efa2075bcb27f4dc43eef749877740d F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654 @@ -116,7 +116,7 @@ F src/date.c 657ff12ca0f1195b531561afacbb38b772d16638 F src/delete.c 2a3d6fc0861b2f8dbd9feb7847b390267b281c60 F src/expr.c c7f3f718bd5c392344ec8694a41c1824f30cf375 F src/fault.c dc88c821842157460750d2d61a8a8b4197d047ff -F src/fkey.c ee4e0d2465c2c092b2b6ab4be7e33ae33d6c6dac +F src/fkey.c 5bbe13f5d9ffba1a21ec16fdc3ac1c0964a28597 F src/func.c e536218d193b8d326aab91120bc4c6f28aa2b606 F src/global.c 271952d199a8cc59d4ce840b3bbbfd2f30c8ba32 F src/hash.c ebcaa921ffd9d86f7ea5ae16a0a29d1c871130a7 @@ -202,7 +202,7 @@ F src/test_thread.c b8a1ab7ca1a632f18e8a361880d5d65eeea08eac F src/test_wsd.c 3ae5101de6cbfda2720152ab659ea84079719241 F src/tokenize.c af8a56e6a50c5042fc305bfa796275e9bf26ff2b F src/trigger.c d2f31617a1852f7365aca54c5c85743b9114d462 -F src/update.c 5df5c39dbe427d6cd5e87c78ae8c1b1d1cb5e304 +F src/update.c a7a5f60bf65a88557b434ac40f2ae6d2f7ad2d2d F src/utf.c 99cf927eabb104621ba889ac0dd075fc1657ad30 F src/util.c 59d4e9456bf1fe581f415a783fa0cee6115c8f35 F src/vacuum.c 869d08eaab64e2a4eaf4ef9ea34b851892b65a75 @@ -330,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 13e99ef0cafe6947638f748aea7b1a7ee8aecb1a +F test/fkey2.test b879ba958126da0bfdfcf5d4532840c0f5956c72 F test/fkey3.test 2183cac9075f3aae4875106eb9255bb73618444e F test/fkey_malloc.test da912d000bb6ceb1cd11b655de1989762fa71ceb F test/format4.test 1f0cac8ff3895e9359ed87e41aaabee982a812eb @@ -755,7 +755,7 @@ F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e F tool/vdbe-compress.tcl d70ea6d8a19e3571d7ab8c9b75cba86d1173ff0f -P 519144ac437b5842e4213f0e81e05c709939c2ab -R 67b182d615cf1918bbb959f1c1561217 +P b4a10c39e726dc190e9597e382baddc034294114 +R 8c38d646c8a9549ba486066fb27ea41b U dan -Z 1c73bf12a1b7498f5704501d82682485 +Z 7018674b123841c492121f6d9b33b5de diff --git a/manifest.uuid b/manifest.uuid index eeb2dc1946..9b01415ba5 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -b4a10c39e726dc190e9597e382baddc034294114 \ No newline at end of file +9e503e2d0428c9e8df878c7c6594790232cca4e0 \ No newline at end of file diff --git a/src/fkey.c b/src/fkey.c index dfd158bfc5..804d1ed977 100644 --- a/src/fkey.c +++ b/src/fkey.c @@ -327,6 +327,7 @@ static void fkLookupParent( if( pIdx==0 ){ /* If pIdx is NULL, then the parent key is the INTEGER PRIMARY KEY ** column of the parent table (table pTab). */ + int iMustBeInt; /* Address of MustBeInt instruction */ int regTemp = sqlite3GetTempReg(pParse); /* Invoke MustBeInt to coerce the child key value to an integer (i.e. @@ -335,16 +336,22 @@ static void fkLookupParent( ** the value. Otherwise, the value inserted into the child key column ** will have INTEGER affinity applied to it, which may not be correct. */ sqlite3VdbeAddOp2(v, OP_SCopy, aiCol[0]+1+regData, regTemp); - sqlite3VdbeAddOp2(v, OP_MustBeInt, regTemp, 0); + iMustBeInt = sqlite3VdbeAddOp2(v, OP_MustBeInt, regTemp, 0); + + /* If the parent table is the same as the child table, and we are about + ** to increment the constraint-counter (i.e. this is an INSERT operation), + ** then check if the row being inserted matches itself. If so, do not + ** increment the constraint-counter. */ + if( pTab==pFKey->pFrom && nIncr==1 ){ + sqlite3VdbeAddOp3(v, OP_Eq, regData, iOk, regTemp); + } + sqlite3OpenTable(pParse, iCur, iDb, pTab, OP_OpenRead); sqlite3VdbeAddOp3(v, OP_NotExists, iCur, 0, regTemp); sqlite3VdbeAddOp2(v, OP_Goto, 0, iOk); sqlite3VdbeJumpHere(v, sqlite3VdbeCurrentAddr(v)-2); - sqlite3VdbeJumpHere(v, sqlite3VdbeCurrentAddr(v)-4); + sqlite3VdbeJumpHere(v, iMustBeInt); sqlite3ReleaseTempReg(pParse, regTemp); - assert( - sqlite3VdbeGetOp(v, sqlite3VdbeCurrentAddr(v)-4)->opcode==OP_MustBeInt - ); }else{ int nCol = pFKey->nCol; int regTemp = sqlite3GetTempRange(pParse, nCol); @@ -353,12 +360,28 @@ static void fkLookupParent( sqlite3VdbeAddOp3(v, OP_OpenRead, iCur, pIdx->tnum, iDb); sqlite3VdbeChangeP4(v, -1, (char*)pKey, P4_KEYINFO_HANDOFF); - for(i=0; ipFrom && nIncr==1 ){ + int iJump = sqlite3VdbeCurrentAddr(v) + nCol + 1; + for(i=0; iaiColumn[i]+1+regData; + sqlite3VdbeAddOp3(v, OP_Ne, iChild, iJump, iParent); + } + sqlite3VdbeAddOp2(v, OP_Goto, 0, iOk); + } + sqlite3VdbeAddOp3(v, OP_MakeRecord, regTemp, nCol, regRec); sqlite3VdbeChangeP4(v, -1, sqlite3IndexAffinityStr(v, pIdx), 0); sqlite3VdbeAddOp3(v, OP_Found, iCur, iOk, regRec); + sqlite3ReleaseTempReg(pParse, regRec); sqlite3ReleaseTempRange(pParse, regTemp, nCol); } @@ -413,6 +436,7 @@ static void fkLookupParent( static void fkScanChildren( Parse *pParse, /* Parse context */ SrcList *pSrc, /* SrcList containing the table to scan */ + Table *pTab, Index *pIdx, /* Foreign key index */ FKey *pFKey, /* Foreign key relationship */ int *aiCol, /* Map from pIdx cols to child table cols */ @@ -427,6 +451,8 @@ static void fkScanChildren( int iFkIfZero = 0; /* Address of OP_FkIfZero */ Vdbe *v = sqlite3GetVdbe(pParse); + assert( !pIdx || pIdx->pTable==pTab ); + if( nIncr<0 ){ iFkIfZero = sqlite3VdbeAddOp2(v, OP_FkIfZero, pFKey->isDeferred, 0); } @@ -469,6 +495,26 @@ static void fkScanChildren( pWhere = sqlite3ExprAnd(db, pWhere, pEq); } + /* If the child table is the same as the parent table, and this scan + ** is taking place as part of a DELETE operation (operation D.2), omit the + ** row being deleted from the scan by adding ($rowid != rowid) to the WHERE + ** clause, where $rowid is the rowid of the row being deleted. */ + if( pTab==pFKey->pFrom && nIncr>0 ){ + Expr *pEq; /* Expression (pLeft = pRight) */ + Expr *pLeft; /* Value from parent table row */ + Expr *pRight; /* Column ref to child table */ + pLeft = sqlite3Expr(db, TK_REGISTER, 0); + pRight = sqlite3Expr(db, TK_COLUMN, 0); + if( pLeft && pRight ){ + pLeft->iTable = regData; + pLeft->affinity = SQLITE_AFF_INTEGER; + pRight->iTable = pSrc->a[0].iCursor; + pRight->iColumn = -1; + } + pEq = sqlite3PExpr(pParse, TK_NE, pLeft, pRight, 0); + pWhere = sqlite3ExprAnd(db, pWhere, pEq); + } + /* Resolve the references in the WHERE clause. */ memset(&sNameContext, 0, sizeof(NameContext)); sNameContext.pSrcList = pSrc; @@ -535,6 +581,7 @@ static void fkTriggerDelete(sqlite3 *dbMem, Trigger *p){ TriggerStep *pStep = p->step_list; sqlite3ExprDelete(dbMem, pStep->pWhere); sqlite3ExprListDelete(dbMem, pStep->pExprList); + sqlite3SelectDelete(dbMem, pStep->pSelect); sqlite3ExprDelete(dbMem, p->pWhen); sqlite3DbFree(dbMem, p); } @@ -551,17 +598,16 @@ static void fkTriggerDelete(sqlite3 *dbMem, Trigger *p){ ** of the row being deleted, from left to right. Parameter regNew is passed ** zero in this case. ** -** For an UPDATE operation, regOld is the first in an array of (pTab->nCol+1) -** registers containing the old rowid and column values of the row being -** updated, and regNew is the first in an array of the same size containing -** the corresponding new values. Parameter pChanges is passed the list of -** columns being updated by the statement. -** ** For an INSERT operation, regOld is passed zero and regNew is passed the ** first register of an array of (pTab->nCol+1) registers containing the new ** row data. ** -** If an error occurs, an error message is left in the pParse structure. +** For an UPDATE operation, this function is called twice. Once before +** the original record is deleted from the table using the calling convention +** described for DELETE. Then again after the original record is deleted +** but before the new record is inserted using the INSERT convention. In +** both cases parameter pChanges is passed the list of columns being +** updated by the statement. */ void sqlite3FkCheck( Parse *pParse, /* Parse context */ @@ -576,7 +622,7 @@ void sqlite3FkCheck( int iDb; /* Index of database containing pTab */ const char *zDb; /* Name of database containing pTab */ - assert( ( pChanges && regOld && regNew) /* UPDATE operation */ + assert( ( pChanges && (regOld==0)!=(regNew==0)) /* UPDATE operation */ || (!pChanges && !regOld && regNew) /* INSERT operation */ || (!pChanges && regOld && !regNew) /* DELETE operation */ ); @@ -678,26 +724,8 @@ void sqlite3FkCheck( pSrc->a->pTab->nRef++; pSrc->a->iCursor = pParse->nTab++; - /* If this is an UPDATE, and none of the columns associated with this - ** FK have been modified, do not scan the child table. Unlike the - ** compile-time test implemented above, this is not just an - ** optimization. It is required so that immediate foreign keys do not - ** throw exceptions when the user executes a statement like: - ** - ** UPDATE refd_table SET refd_column = refd_column - */ - if( pChanges ){ - int i; - int iJump = sqlite3VdbeCurrentAddr(v) + pFKey->nCol + 1; - for(i=0; inCol; i++){ - int iOff = (pIdx ? pIdx->aiColumn[i] : -1) + 1; - sqlite3VdbeAddOp3(v, OP_Ne, regOld+iOff, iJump, regNew+iOff); - } - iGoto = sqlite3VdbeAddOp0(v, OP_Goto); - } - if( regNew!=0 ){ - fkScanChildren(pParse, pSrc, pIdx, pFKey, aiCol, regNew, -1); + fkScanChildren(pParse, pSrc, pTab, pIdx, pFKey, aiCol, regNew, -1); } if( regOld!=0 ){ /* If there is a RESTRICT action configured for the current operation @@ -706,9 +734,7 @@ void sqlite3FkCheck( ** deferred trigger. That's what RESTRICT means. To defer checking ** the constraint, the FK should specify NO ACTION (represented ** using OE_None). NO ACTION is the default. */ - fkScanChildren(pParse, pSrc, pIdx, pFKey, aiCol, regOld, - pFKey->aAction[pChanges!=0]!=OE_Restrict - ); + fkScanChildren(pParse, pSrc, pTab, pIdx, pFKey, aiCol, regOld, 1); } if( pChanges ){ @@ -815,10 +841,7 @@ static Trigger *fkActionTrigger( action = pFKey->aAction[iAction]; pTrigger = pFKey->apTrigger[iAction]; - assert( OE_SetNull>OE_Restrict && OE_SetDflt>OE_Restrict ); - assert( OE_Cascade>OE_Restrict && OE_NoneOE_Restrict && !pTrigger ){ + if( action!=OE_None && !pTrigger ){ u8 enableLookaside; /* Copy of db->lookaside.bEnabled */ char const *zFrom; /* Name of child table */ int nFrom; /* Length in bytes of zFrom */ @@ -827,6 +850,7 @@ static Trigger *fkActionTrigger( TriggerStep *pStep; /* First (only) step of trigger program */ Expr *pWhere = 0; /* WHERE clause of trigger step */ ExprList *pList = 0; /* Changes list if ON UPDATE CASCADE */ + Select *pSelect = 0; /* If RESTRICT, "SELECT RAISE(...)" */ int i; /* Iterator variable */ Expr *pWhen = 0; /* WHEN clause for the trigger */ @@ -878,7 +902,7 @@ static Trigger *fkActionTrigger( pWhen = sqlite3ExprAnd(db, pWhen, pEq); } - if( action!=OE_Cascade || pChanges ){ + if( action!=OE_Restrict && (action!=OE_Cascade || pChanges) ){ Expr *pNew; if( action==OE_Cascade ){ pNew = sqlite3PExpr(pParse, TK_DOT, @@ -901,6 +925,28 @@ static Trigger *fkActionTrigger( } sqlite3DbFree(db, aiCol); + zFrom = pFKey->pFrom->zName; + nFrom = sqlite3Strlen30(zFrom); + + if( action==OE_Restrict ){ + Token tFrom; + Expr *pRaise; + + tFrom.z = zFrom; + tFrom.n = nFrom; + pRaise = sqlite3Expr(db, TK_RAISE, "foreign key constraint failed"); + if( pRaise ){ + pRaise->affinity = OE_Abort; + } + pSelect = sqlite3SelectNew(pParse, + sqlite3ExprListAppend(pParse, 0, pRaise), + sqlite3SrcListAppend(db, 0, &tFrom, 0), + pWhere, + 0, 0, 0, 0, 0, 0 + ); + pWhere = 0; + } + /* In the current implementation, pTab->dbMem==0 for all tables except ** for temporary tables used to describe subqueries. And temporary ** tables do not have foreign key constraints. Hence, pTab->dbMem @@ -909,8 +955,6 @@ static Trigger *fkActionTrigger( enableLookaside = db->lookaside.bEnabled; db->lookaside.bEnabled = 0; - zFrom = pFKey->pFrom->zName; - nFrom = sqlite3Strlen30(zFrom); pTrigger = (Trigger *)sqlite3DbMallocZero(db, sizeof(Trigger) + /* struct Trigger */ sizeof(TriggerStep) + /* Single step in trigger program */ @@ -924,6 +968,7 @@ static Trigger *fkActionTrigger( pStep->pWhere = sqlite3ExprDup(db, pWhere, EXPRDUP_REDUCE); pStep->pExprList = sqlite3ExprListDup(db, pList, EXPRDUP_REDUCE); + pStep->pSelect = sqlite3SelectDup(db, pSelect, EXPRDUP_REDUCE); if( pWhen ){ pWhen = sqlite3PExpr(pParse, TK_NOT, pWhen, 0, 0); pTrigger->pWhen = sqlite3ExprDup(db, pWhen, EXPRDUP_REDUCE); @@ -936,12 +981,24 @@ static Trigger *fkActionTrigger( sqlite3ExprDelete(db, pWhere); sqlite3ExprDelete(db, pWhen); sqlite3ExprListDelete(db, pList); + sqlite3SelectDelete(db, pSelect); if( db->mallocFailed==1 ){ fkTriggerDelete(db, pTrigger); return 0; } - pStep->op = (action!=OE_Cascade || pChanges) ? TK_UPDATE : TK_DELETE; + switch( action ){ + case OE_Restrict: + pStep->op = TK_SELECT; + break; + case OE_Cascade: + if( !pChanges ){ + pStep->op = TK_DELETE; + break; + } + default: + pStep->op = TK_UPDATE; + } pStep->pTrig = pTrigger; pTrigger->pSchema = pTab->pSchema; pTrigger->pTabSchema = pTab->pSchema; diff --git a/src/update.c b/src/update.c index 4fcfaeae48..f443dbc492 100644 --- a/src/update.c +++ b/src/update.c @@ -445,17 +445,19 @@ void sqlite3Update( aRegIdx, (chngRowid?regOldRowid:0), 1, onError, addr, 0); /* Do FK constraint checks. */ - sqlite3FkCheck(pParse, pTab, pChanges, regOldRowid, regNewRowid); + sqlite3FkCheck(pParse, pTab, pChanges, regOldRowid, 0); /* Delete the index entries associated with the current record. */ j1 = sqlite3VdbeAddOp3(v, OP_NotExists, iCur, 0, regOldRowid); sqlite3GenerateRowIndexDelete(pParse, pTab, iCur, aRegIdx); /* If changing the record number, delete the old record. */ - if( chngRowid ){ + if( hasFK || chngRowid ){ sqlite3VdbeAddOp2(v, OP_Delete, iCur, 0); } sqlite3VdbeJumpHere(v, j1); + + sqlite3FkCheck(pParse, pTab, pChanges, 0, regNewRowid); /* Insert the new index entries and the new record. */ sqlite3CompleteInsertion(pParse, pTab, iCur, regNewRowid, aRegIdx, 1, 0, 0); diff --git a/test/fkey2.test b/test/fkey2.test index 0fb4218f6b..ed74bdeacb 100644 --- a/test/fkey2.test +++ b/test/fkey2.test @@ -65,6 +65,9 @@ ifcapable {!foreignkey||!trigger} { # table or deleting from a child table does not cause SQLite # to check if this has repaired an outstanding violation. # +# fkey2-16.*: Test that rows that refer to themselves may be inserted, +# updated and deleted. +# # fkey2-genfkey.*: Tests that were used with the shell tool .genfkey # command. Recycled to test the built-in implementation. # @@ -82,6 +85,18 @@ proc drop_all_tables {{db db}} { execsql { PRAGMA foreign_keys = on } +if 0 { +execsql { + CREATE TABLE t1(a PRIMARY KEY, b REFERENCES t1); + INSERT INTO t1 VALUES('aaa', 'aaa'); +} +puts XXX +explain { UPDATE t1 SET a = 'bbb' } +execsql { PRAGMA vdbe_trace = 1 } +execsql { UPDATE t1 SET a = 'bbb' } +exit +} + set FkeySimpleSchema { PRAGMA foreign_keys = on; CREATE TABLE t1(a PRIMARY KEY, b); @@ -915,6 +930,48 @@ do_test fkey2-15.1.7 { } } {2} +#------------------------------------------------------------------------- +# This next block of tests, fkey2-16.*, test that rows that refer to +# themselves may be inserted and deleted. +# +foreach {tn zSchema} { + 1 { CREATE TABLE self(a INTEGER PRIMARY KEY, b REFERENCES self(a)) } + 2 { CREATE TABLE self(a PRIMARY KEY, b REFERENCES self(a)) } + 3 { CREATE TABLE self(a UNIQUE, b INTEGER PRIMARY KEY REFERENCES self(a)) } +} { + drop_all_tables + do_test fkey2-16.1.$tn.1 { + execsql $zSchema + execsql { INSERT INTO self VALUES(13, 13) } + } {} + do_test fkey2-16.1.$tn.2 { + execsql { UPDATE self SET a = 14, b = 14 } + } {} + + do_test fkey2-16.1.$tn.3 { + catchsql { UPDATE self SET b = 15 } + } {1 {foreign key constraint failed}} + + do_test fkey2-16.1.$tn.4 { + catchsql { UPDATE self SET a = 15 } + } {1 {foreign key constraint failed}} + + do_test fkey2-16.1.$tn.5 { + catchsql { UPDATE self SET a = 15, b = 16 } + } {1 {foreign key constraint failed}} + + do_test fkey2-16.1.$tn.6 { + catchsql { UPDATE self SET a = 17, b = 17 } + } {0 {}} + + do_test fkey2-16.1.$tn.7 { + execsql { DELETE FROM self } + } {} + do_test fkey2-16.1.$tn.8 { + catchsql { INSERT INTO self VALUES(20, 21) } + } {1 {foreign key constraint failed}} +} + #------------------------------------------------------------------------- # The following block of tests, those prefixed with "fkey2-genfkey.", are