From 0ff297eaa75ac3a7b52cdc7a77411ac2e6f9fdb4 Mon Sep 17 00:00:00 2001 From: dan Date: Fri, 25 Sep 2009 17:03:14 +0000 Subject: [PATCH] Avoid checking if an insert or delete has "fixed" an outstanding FK constraint violation if the constraint counter indicates that the database contains no such violations. FossilOrigin-Name: 519144ac437b5842e4213f0e81e05c709939c2ab --- manifest | 20 ++++++------- manifest.uuid | 2 +- src/fkey.c | 26 +++++++++++++---- src/test1.c | 3 ++ src/vdbe.c | 47 ++++++++++++++++++++++++++---- src/vdbeaux.c | 2 +- test/fkey2.test | 76 +++++++++++++++++++++++++++++++++++++++++++++---- 7 files changed, 147 insertions(+), 29 deletions(-) diff --git a/manifest b/manifest index f4a6ddd82c..5a1ee5aca2 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Fix\scomments\sin\sfkey2.c\sto\sreflect\sthe\simmediate-constraint-counter\sapproach. -D 2009-09-25T12:00:02 +C Avoid\schecking\sif\san\sinsert\sor\sdelete\shas\s"fixed"\san\soutstanding\sFK\sconstraint\sviolation\sif\sthe\sconstraint\scounter\sindicates\sthat\sthe\sdatabase\scontains\sno\ssuch\sviolations. +D 2009-09-25T17:03:14 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 8719d0fa09f8f08d5b21abf54a59cbcb88172b1b +F src/fkey.c 542de974e3ed3a507f00b68d1486a4cbeaf4d8bb F src/func.c e536218d193b8d326aab91120bc4c6f28aa2b606 F src/global.c 271952d199a8cc59d4ce840b3bbbfd2f30c8ba32 F src/hash.c ebcaa921ffd9d86f7ea5ae16a0a29d1c871130a7 @@ -169,7 +169,7 @@ F src/sqliteLimit.h 504a3161886d2938cbd163054ad620b8356df758 F src/status.c 237b193efae0cf6ac3f0817a208de6c6c6ef6d76 F src/table.c cc86ad3d6ad54df7c63a3e807b5783c90411a08d F src/tclsqlite.c 5eea5025c370d3a91ce0415f9d46f96fdc7aef44 -F src/test1.c 83a685fa2c96f005934ed09146b53522b1aa533e +F src/test1.c 9bd64834314b67345855c314dc479bc12596a9b7 F src/test2.c 0de743ec8890ca4f09e0bce5d6d5a681f5957fec F src/test3.c 2445c2beb5e7a0c91fd8136dc1339ec369a24898 F src/test4.c b5fd530f02a6a0dbffb23be202168a690985dedd @@ -206,11 +206,11 @@ F src/update.c 5df5c39dbe427d6cd5e87c78ae8c1b1d1cb5e304 F src/utf.c 99cf927eabb104621ba889ac0dd075fc1657ad30 F src/util.c 59d4e9456bf1fe581f415a783fa0cee6115c8f35 F src/vacuum.c 869d08eaab64e2a4eaf4ef9ea34b851892b65a75 -F src/vdbe.c a5da14fe8d89f9ad2cd4911a9d7df79c74a6b84c +F src/vdbe.c cb9dae4c5c3706e5772756bb38710faa954f0bb9 F src/vdbe.h 7d5075e3fa4e5587a9be8d5e503857c825490cef F src/vdbeInt.h 7afb76c0296f9a2310e565803fa66798ef47e9d5 F src/vdbeapi.c 524d79eb17bbcbe31c37c908b8e01edc5c684a90 -F src/vdbeaux.c c36bb6674d43c8a1f553648c15fcd078c7357262 +F src/vdbeaux.c 6834737c119f5662c9e6d147ddb4f72523a31aea F src/vdbeblob.c 3ba0f7ba1b3afce2d37a18e4f437992d430f0eae F src/vdbemem.c 0ff2b209fccade3ff6709286057b82ed7f6c1e70 F src/vtab.c 3e54fe39374e5feb8b174de32a90e7a21966025d @@ -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 8f857439feef71b11a13821aa407399e3ed9f494 +F test/fkey2.test 4b22c954c3923ffe366feb664b33b1b438abb5b1 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 353b1b18253ab71ba38a887e555994f5469b87bd -R 686fb8f7dc051b5c786ec23c8c348ab4 +P 9fd54b0aa73ed74c65f7db53cb666752f13263f9 +R 5e90fcf7b135a7c55fc18c54261d65ee U dan -Z 007854d1d118a1cdcf5b42c76aa6bc04 +Z 1aaee7b75a94b33f2eb5b3e61baf845b diff --git a/manifest.uuid b/manifest.uuid index f4a31f8d70..39275a4071 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -9fd54b0aa73ed74c65f7db53cb666752f13263f9 \ No newline at end of file +519144ac437b5842e4213f0e81e05c709939c2ab \ No newline at end of file diff --git a/src/fkey.c b/src/fkey.c index acac588cf4..e7250d9a43 100644 --- a/src/fkey.c +++ b/src/fkey.c @@ -309,9 +309,16 @@ static void fkLookupParent( int iCur = pParse->nTab - 1; /* Cursor number to use */ int iOk = sqlite3VdbeMakeLabel(v); /* jump here if parent key found */ - /* 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. */ + /* If nIncr is less than zero, then check at runtime if there are any + ** outstanding constraints to resolve. If there are not, there is no need + ** to check if deleting this row resolves any outstanding violations. + ** + ** Check if any of the key columns in the child table row are NULL. If + ** any are, then the constraint is considered satisfied. No need to + ** search for a matching row in the parent table. */ + if( nIncr<0 ){ + sqlite3VdbeAddOp2(v, OP_FkIfZero, pFKey->isDeferred, iOk); + } for(i=0; inCol; i++){ int iReg = aiCol[i] + regData + 1; sqlite3VdbeAddOp2(v, OP_IsNull, iReg, iOk); @@ -369,7 +376,7 @@ static void fkLookupParent( if( nIncr>0 && pFKey->isDeferred==0 ){ sqlite3ParseToplevel(pParse)->mayAbort = 1; } - sqlite3VdbeAddOp2(v, OP_FkCounter, nIncr, pFKey->isDeferred); + sqlite3VdbeAddOp2(v, OP_FkCounter, pFKey->isDeferred, nIncr); } sqlite3VdbeResolveLabel(v, iOk); @@ -417,6 +424,12 @@ static void fkScanChildren( Expr *pWhere = 0; /* WHERE clause to scan with */ NameContext sNameContext; /* Context used to resolve WHERE clause */ WhereInfo *pWInfo; /* Context used by sqlite3WhereXXX() */ + int iFkIfZero = 0; /* Address of OP_FkIfZero */ + Vdbe *v = sqlite3GetVdbe(pParse); + + if( nIncr<0 ){ + iFkIfZero = sqlite3VdbeAddOp2(v, OP_FkIfZero, pFKey->isDeferred, 0); + } /* Create an Expr object representing an SQL expression like: ** @@ -477,7 +490,7 @@ static void fkScanChildren( if( nIncr>0 && pFKey->isDeferred==0 ){ sqlite3ParseToplevel(pParse)->mayAbort = 1; } - sqlite3VdbeAddOp2(pParse->pVdbe, OP_FkCounter, nIncr, pFKey->isDeferred); + sqlite3VdbeAddOp2(v, OP_FkCounter, pFKey->isDeferred, nIncr); } if( pWInfo ){ sqlite3WhereEnd(pWInfo); @@ -485,6 +498,9 @@ static void fkScanChildren( /* Clean up the WHERE clause constructed above. */ sqlite3ExprDelete(db, pWhere); + if( iFkIfZero ){ + sqlite3VdbeJumpHere(v, iFkIfZero); + } } /* diff --git a/src/test1.c b/src/test1.c index e46ddfdd3e..aa28a3dfff 100644 --- a/src/test1.c +++ b/src/test1.c @@ -4873,6 +4873,7 @@ static int test_unlock_notify( */ int Sqlitetest1_Init(Tcl_Interp *interp){ extern int sqlite3_search_count; + extern int sqlite3_found_count; extern int sqlite3_interrupt_count; extern int sqlite3_open_file_count; extern int sqlite3_sort_count; @@ -5093,6 +5094,8 @@ int Sqlitetest1_Init(Tcl_Interp *interp){ } Tcl_LinkVar(interp, "sqlite_search_count", (char*)&sqlite3_search_count, TCL_LINK_INT); + Tcl_LinkVar(interp, "sqlite_found_count", + (char*)&sqlite3_found_count, TCL_LINK_INT); Tcl_LinkVar(interp, "sqlite_sort_count", (char*)&sqlite3_sort_count, TCL_LINK_INT); Tcl_LinkVar(interp, "sqlite3_max_blobsize", diff --git a/src/vdbe.c b/src/vdbe.c index 6014589ae4..f5cd7ce77a 100644 --- a/src/vdbe.c +++ b/src/vdbe.c @@ -98,6 +98,17 @@ static void updateMaxBlobsize(Mem *p){ } #endif +/* +** The next global variable is incremented each type the OP_Found opcode +** is executed. This is used to test whether or not the foreign key +** operation implemented using OP_FkIsZero is working. This variable +** has no function other than to help verify the correct operation of the +** library. +*/ +#ifdef SQLITE_TEST +int sqlite3_found_count = 0; +#endif + /* ** Test a register to see if it exceeds the current maximum blob size. ** If it does, record the new maximum blob size. @@ -3379,6 +3390,10 @@ case OP_Found: { /* jump, in3 */ UnpackedRecord *pIdxKey; char aTempRec[ROUND8(sizeof(UnpackedRecord)) + sizeof(Mem)*3 + 7]; +#ifdef SQLITE_TEST + sqlite3_found_count++; +#endif + alreadyExists = 0; assert( pOp->p1>=0 && pOp->p1nCursor ); pC = p->apCsr[pOp->p1]; @@ -4902,16 +4917,36 @@ case OP_Param: { /* out2-prerelease */ #ifndef SQLITE_OMIT_FOREIGN_KEY /* Opcode: FkCounter P1 P2 * * * ** -** 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 +** Increment a "constraint counter" by P2 (P2 may be negative or positive). +** If P1 is non-zero, the database constraint counter is incremented +** (deferred foreign key constraints). Otherwise, if P1 is zero, the ** statement counter is incremented (immediate foreign key constraints). */ case OP_FkCounter: { - if( pOp->p2 ){ - db->nDeferredCons += pOp->p1; + if( pOp->p1 ){ + db->nDeferredCons += pOp->p2; }else{ - p->nFkConstraint += pOp->p1; + p->nFkConstraint += pOp->p2; + } + break; +} + +/* Opcode: FkIfZero P1 P2 * * * +** +** This opcode tests if a foreign key constraint-counter is currently zero. +** If so, jump to instruction P2. Otherwise, fall through to the next +** instruction. +** +** If P1 is non-zero, then the jump is taken if the database constraint-counter +** is zero (the one that counts deferred constraint violations). If P1 is +** zero, the jump is taken if the statement constraint-counter is zero +** (immediate foreign key constraint violations). +*/ +case OP_FkIfZero: { /* jump */ + if( pOp->p1 ){ + if( db->nDeferredCons==0 ) pc = pOp->p2-1; + }else{ + if( p->nFkConstraint==0 ) pc = pOp->p2-1; } break; } diff --git a/src/vdbeaux.c b/src/vdbeaux.c index ad298d6461..3db26b29e0 100644 --- a/src/vdbeaux.c +++ b/src/vdbeaux.c @@ -341,7 +341,7 @@ int sqlite3VdbeAssertMayAbort(Vdbe *v, int mayAbort){ 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) + || (opcode==OP_FkCounter && pOp->p1==0 && pOp->p2==1) #endif || ((opcode==OP_Halt || opcode==OP_HaltIfNull) && (pOp->p1==SQLITE_CONSTRAINT && pOp->p2==OE_Abort)) diff --git a/test/fkey2.test b/test/fkey2.test index 0b4402c7d5..6508d37c5e 100644 --- a/test/fkey2.test +++ b/test/fkey2.test @@ -60,6 +60,11 @@ ifcapable {!foreignkey||!trigger} { # # fkey2-14.*: Test the ALTER TABLE and DROP TABLE commands. # +# fkey2-15.*: Test that if there are no (known) outstanding foreign key +# constraint violations in the database, inserting into a parent +# table or deleting from a child table does not cause SQLite +# to check if this has repaired an outstanding violation. +# # fkey2-genfkey.*: Tests that were used with the shell tool .genfkey # command. Recycled to test the built-in implementation. # @@ -91,6 +96,8 @@ set FkeySimpleSchema { CREATE TABLE t9(a REFERENCES nosuchtable, b); CREATE TABLE t10(a REFERENCES t9(c) /D/, b); } + + set FkeySimpleTests { 1.1 "INSERT INTO t2 VALUES(1, 3)" {1 {foreign key constraint failed}} 1.2 "INSERT INTO t1 VALUES(1, 2)" {0 {}} @@ -768,7 +775,7 @@ do_test fkey2-13.1.4 { # fkey2-14.3*: DROP TABLE # drop_all_tables -do_test fkey2-14.1 { +do_test fkey2-14.1.1 { # Adding a column with a REFERENCES clause is not supported. execsql { CREATE TABLE t1(a PRIMARY KEY); @@ -776,19 +783,19 @@ do_test fkey2-14.1 { } catchsql { ALTER TABLE t2 ADD COLUMN c REFERENCES t1 } } {0 {}} -do_test fkey2-14.2 { +do_test fkey2-14.1.2 { catchsql { ALTER TABLE t2 ADD COLUMN d DEFAULT NULL REFERENCES t1 } } {0 {}} -do_test fkey2-14.3 { +do_test fkey2-14.1.3 { catchsql { ALTER TABLE t2 ADD COLUMN e REFERENCES t1 DEFAULT NULL} } {0 {}} -do_test fkey2-14.4 { +do_test fkey2-14.1.4 { catchsql { ALTER TABLE t2 ADD COLUMN f REFERENCES t1 DEFAULT 'text'} } {1 {Cannot add a REFERENCES column with non-NULL default value}} -do_test fkey2-14.5 { +do_test fkey2-14.1.5 { catchsql { ALTER TABLE t2 ADD COLUMN g DEFAULT CURRENT_TIME REFERENCES t1 } } {1 {Cannot add a REFERENCES column with non-NULL default value}} -do_test fkey2-14.5 { +do_test fkey2-14.1.6 { execsql { PRAGMA foreign_keys = off; ALTER TABLE t2 ADD COLUMN h DEFAULT 'text' REFERENCES t1; @@ -797,6 +804,63 @@ do_test fkey2-14.5 { } } {{CREATE TABLE t2(a, b, c REFERENCES t1, d DEFAULT NULL REFERENCES t1, e REFERENCES t1 DEFAULT NULL, h DEFAULT 'text' REFERENCES t1)}} +#------------------------------------------------------------------------- +# The following tests, fkey2-15.*, test that unnecessary FK related scans +# and lookups are avoided when the constraint counters are zero. +# +drop_all_tables +proc execsqlS {zSql} { + set ::sqlite_search_count 0 + set ::sqlite_found_count 0 + set res [uplevel [list execsql $zSql]] + concat [expr $::sqlite_found_count + $::sqlite_search_count] $res +} +do_test fkey2-15.1.1 { + execsql { + CREATE TABLE pp(a PRIMARY KEY, b); + CREATE TABLE cc(x, y REFERENCES pp DEFERRABLE INITIALLY DEFERRED); + INSERT INTO pp VALUES(1, 'one'); + INSERT INTO pp VALUES(2, 'two'); + INSERT INTO cc VALUES('neung', 1); + INSERT INTO cc VALUES('song', 2); + } +} {} +do_test fkey2-15.1.2 { + execsqlS { INSERT INTO pp VALUES(3, 'three') } +} {0} +do_test fkey2-15.1.3 { + execsql { + BEGIN; + INSERT INTO cc VALUES('see', 4); -- Violates deferred constraint + } + execsqlS { INSERT INTO pp VALUES(5, 'five') } +} {2} +do_test fkey2-15.1.4 { + execsql { DELETE FROM cc WHERE x = 'see' } + execsqlS { INSERT INTO pp VALUES(6, 'six') } +} {0} +do_test fkey2-15.1.5 { + execsql COMMIT +} {} +do_test fkey2-15.1.6 { + execsql BEGIN + execsqlS { + DELETE FROM cc WHERE x = 'neung'; + ROLLBACK; + } +} {1} +do_test fkey2-15.1.7 { + execsql { + BEGIN; + DELETE FROM pp WHERE a = 2; + } + execsqlS { + DELETE FROM cc WHERE x = 'neung'; + ROLLBACK; + } +} {2} + + #------------------------------------------------------------------------- # The following block of tests, those prefixed with "fkey2-genfkey.", are # the same tests that were used to test the ".genfkey" command provided