From: drh Date: Sat, 26 Oct 2019 00:04:21 +0000 (+0000) Subject: If replace triggers are run during uniqueness checking, then rerun all X-Git-Tag: version-3.31.0~383 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a407eccb6179ee033b8e83a6eab4734dd075664b;p=thirdparty%2Fsqlite.git If replace triggers are run during uniqueness checking, then rerun all uniqueness checks a second time using the ABORT algorithm. Fix for ticket [c1e19e12046d23fe] FossilOrigin-Name: fbac0c65d8464b126d385262d176864add55452ec9e3d5eb76ffee06e820cb9c --- diff --git a/manifest b/manifest index ca33b71ebd..fd044ebade 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Performance\soptimization\sin\ssqlite3BtreeCursor(). -D 2019-10-25T14:46:05.015 +C If\sreplace\striggers\sare\srun\sduring\suniqueness\schecking,\sthen\srerun\sall\nuniqueness\schecks\sa\ssecond\stime\susing\sthe\sABORT\salgorithm.\nFix\sfor\sticket\s[c1e19e12046d23fe] +D 2019-10-26T00:04:21.466 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -487,7 +487,7 @@ F src/hash.c 8d7dda241d0ebdafb6ffdeda3149a412d7df75102cecfc1021c98d6219823b19 F src/hash.h 9d56a9079d523b648774c1784b74b89bd93fac7b365210157482e4319a468f38 F src/hwtime.h 747c1bbe9df21a92e9c50f3bbec1de841dc5e5da F src/in-operator.md 10cd8f4bcd225a32518407c2fb2484089112fd71 -F src/insert.c 2b930afc0fb4c245916d19f638e4b332f4a6d8a3a7c73c288ec18e25caf46900 +F src/insert.c 83ccd3f1f45f048bca2dbc0f9c34e8f3a57fdcbc43ddec04a7938090ffb85a9e F src/legacy.c d7874bc885906868cd51e6c2156698f2754f02d9eee1bae2d687323c3ca8e5aa F src/loadext.c 4ddc65ae13c0d93db0ceedc8b14a28c8c260513448b0eb8c5a2ac375e3b6a85d F src/main.c 3e01f6a1c96643381b5f9d79e4ff7f2520bc5712197746fb0852283e78cccf66 @@ -1061,7 +1061,7 @@ F test/indexexpr1.test c26c8b352311c1deb30642cd0379e5cb94e416c7e9e0885e92d9e0155 F test/indexexpr2.test b580f378423bca443ffab47ada677203cfcf8a60f48a8aa20065f27c8f7739b5 F test/indexfault.test 98d78a8ff1f5335628b62f886a1cb7c7dac1ef6d48fa39c51ec871c87dce9811 F test/init.test 15c823093fdabbf7b531fe22cf037134d09587a7 -F test/insert.test c0e1b23f6359e06316b3a49f7747c5d65c0b6473619011e4fb86f6801edba6df +F test/insert.test d15bf96ec2120ecd27cb07be2b44b141a98b4b7f58762a40178f8205f9b9a96a F test/insert2.test 4d14b8f1b810a41995f6286b64a6943215d52208 F test/insert3.test 1b7db95a03ad9c5013fdf7d6722b6cd66ee55e30 F test/insert4.test 7802ada6ba8738661b9f6c0e26858d3375b40cc7180289fd350644cd7a08fec9 @@ -1848,7 +1848,7 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0 -P 8d964e1c21d4cea699023e02b0616a75c5859dd083c9365cdcbc0676ebbdaae4 -R 8536c2fabd9f7098038221269ecc3472 +P ea068b099c96b8b9526114732d2a6be186cf381b7329d102778ad25b95510c9e +R cace6824aa8651d52f54d321985ad4c0 U drh -Z ac1675b2b1cf8ed8d7717d3ccad9b463 +Z 1fcf2849ed7e21c156487e77513e1cff diff --git a/manifest.uuid b/manifest.uuid index 9f2d452b24..62410d7fa7 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -ea068b099c96b8b9526114732d2a6be186cf381b7329d102778ad25b95510c9e \ No newline at end of file +fbac0c65d8464b126d385262d176864add55452ec9e3d5eb76ffee06e820cb9c \ No newline at end of file diff --git a/src/insert.c b/src/insert.c index 77b402c7ec..e435f4363d 100644 --- a/src/insert.c +++ b/src/insert.c @@ -1319,6 +1319,13 @@ void sqlite3GenerateConstraintChecks( int upsertJump = 0; /* Address of Goto that jumps into upsert subroutine */ int ipkTop = 0; /* Top of the IPK uniqueness check */ int ipkBottom = 0; /* OP_Goto at the end of the IPK uniqueness check */ + /* Variables associated with retesting uniqueness constraints after + ** replace triggers fire have run */ + int regTrigCnt; /* Register used to count replace trigger invocations */ + int addrRecheck = 0; /* Jump here to recheck all uniqueness constraints */ + int lblRecheckOk = 0; /* Each recheck jumps to this label if it passes */ + Trigger *pTrigger; /* List of DELETE triggers on the table pTab */ + int nReplaceTrig = 0; /* Number of replace triggers coded */ isUpdate = regOldData!=0; db = pParse->db; @@ -1483,6 +1490,50 @@ void sqlite3GenerateConstraintChecks( } } + /* Determine if it is possible that triggers (either explicitly coded + ** triggers or FK resolution actions) might run as a result of deletes + ** that happen when OE_Replace conflict resolution occurs. (Call these + ** "replace triggers".) If any replace triggers run, we will need to + ** recheck all of the uniqueness constraints after they have all run. + ** But on the recheck, the resolution is OE_Abort instead of OE_Replace. + ** + ** If replace triggers are a possibility, then + ** + ** (1) Allocate register regTrigCnt and initialize it to zero. + ** That register will count the number of replace triggers that + ** fire. Constraint recheck only occurs if the number if positive. + ** (2) Initialize pTrigger to the set of all DELETE triggers. + ** (3) Initialize addrRecheck and lblRecheckOk + ** + ** The uniqueness rechecking code will create a series of tests to run + ** in a second pass. The addrRecheck and lblRecheckOk variables are + ** used to link together these tests which are separated from each other + ** in the generate bytecode. + */ + if( (db->flags & (SQLITE_RecTriggers|SQLITE_ForeignKeys))==0 ){ + /* There are not DELETE triggers nor FK constraints. No constraint + ** rechecks are needed. */ + pTrigger = 0; + regTrigCnt = 0; + }else{ + if( db->flags&SQLITE_RecTriggers ){ + pTrigger = sqlite3TriggersExist(pParse, pTab, TK_DELETE, 0, 0); + regTrigCnt = pTrigger!=0 || sqlite3FkRequired(pParse, pTab, 0, 0); + }else{ + pTrigger = 0; + regTrigCnt = sqlite3FkRequired(pParse, pTab, 0, 0); + } + if( regTrigCnt ){ + /* Replace triggers might exist. Allocate the counter and + ** initialize it to zero. */ + regTrigCnt = ++pParse->nMem; + sqlite3VdbeAddOp2(v, OP_Integer, 0, regTrigCnt); + VdbeComment((v, "trigger count")); + lblRecheckOk = sqlite3VdbeMakeLabel(pParse); + addrRecheck = lblRecheckOk; + } + } + /* If rowid is changing, make sure the new rowid does not previously ** exist in the table. */ @@ -1572,17 +1623,12 @@ void sqlite3GenerateConstraintChecks( ** to run without a statement journal if there are no indexes on the ** table. */ - Trigger *pTrigger = 0; - if( db->flags&SQLITE_RecTriggers ){ - pTrigger = sqlite3TriggersExist(pParse, pTab, TK_DELETE, 0, 0); - } - if( pTrigger || sqlite3FkRequired(pParse, pTab, 0, 0) ){ + if( regTrigCnt ){ sqlite3MultiWrite(pParse); sqlite3GenerateRowDelete(pParse, pTab, pTrigger, iDataCur, iIdxCur, regNewData, 1, 0, OE_Replace, 1, -1); - sqlite3VdbeAddOp3(v, OP_NotExists, iDataCur, addrRowidOk, regNewData); - VdbeCoverage(v); - sqlite3RowidConstraint(pParse, OE_Abort, pTab); + sqlite3VdbeAddOp2(v, OP_AddImm, regTrigCnt, 1); /* incr trigger cnt */ + nReplaceTrig++; }else{ #ifdef SQLITE_ENABLE_PREUPDATE_HOOK assert( HasRowid(pTab) ); @@ -1632,6 +1678,7 @@ void sqlite3GenerateConstraintChecks( int regR; /* Range of registers holding conflicting PK */ int iThisCur; /* Cursor for this UNIQUE index */ int addrUniqueOk; /* Jump here if the UNIQUE constraint is satisfied */ + int addrConflictCk; /* First opcode in the conflict check logic */ if( aRegIdx[ix]==0 ) continue; /* Skip indices that do not change */ if( pUpIdx==pIdx ){ @@ -1745,8 +1792,9 @@ void sqlite3GenerateConstraintChecks( /* Check to see if the new index entry will be unique */ sqlite3VdbeVerifyAbortable(v, onError); - sqlite3VdbeAddOp4Int(v, OP_NoConflict, iThisCur, addrUniqueOk, - regIdx, pIdx->nKeyCol); VdbeCoverage(v); + addrConflictCk = + sqlite3VdbeAddOp4Int(v, OP_NoConflict, iThisCur, addrUniqueOk, + regIdx, pIdx->nKeyCol); VdbeCoverage(v); /* Generate code to handle collisions */ regR = (pIdx==pPk) ? regIdx : sqlite3GetTempRange(pParse, nPkField); @@ -1829,23 +1877,59 @@ void sqlite3GenerateConstraintChecks( break; } default: { - Trigger *pTrigger = 0; - int bRetryConstraintCheck = 0; + int nConflictCk; /* Number of opcodes in conflict check logic */ + assert( onError==OE_Replace ); - if( db->flags&SQLITE_RecTriggers ){ - pTrigger = sqlite3TriggersExist(pParse, pTab, TK_DELETE, 0, 0); - } - if( pTrigger || sqlite3FkRequired(pParse, pTab, 0, 0) ){ + nConflictCk = sqlite3VdbeCurrentAddr(v) - addrConflictCk; + if( regTrigCnt ){ sqlite3MultiWrite(pParse); - bRetryConstraintCheck = 1; + nReplaceTrig++; } sqlite3GenerateRowDelete(pParse, pTab, pTrigger, iDataCur, iIdxCur, regR, nPkField, 0, OE_Replace, (pIdx==pPk ? ONEPASS_SINGLE : ONEPASS_OFF), iThisCur); - if( bRetryConstraintCheck ){ - sqlite3VdbeAddOp4Int(v, OP_NoConflict, iThisCur, addrUniqueOk, - regIdx, pIdx->nKeyCol); VdbeCoverage(v); + if( regTrigCnt ){ + VdbeOp *pOp; /* Conflict check opcode to copy */ + int p2; /* New P2 value for copied conflict check opcode */ + int addrBypass; /* Jump destination to bypass recheck logic */ + + sqlite3VdbeAddOp2(v, OP_AddImm, regTrigCnt, 1); /* incr trigger cnt */ + addrBypass = sqlite3VdbeAddOp0(v, OP_Goto); /* Bypass recheck */ + VdbeComment((v, "bypass recheck")); + + /* Here we insert code that will be invoked after all constraint + ** checks have run, if and only if one or more replace triggers + ** fired. */ + sqlite3VdbeResolveLabel(v, lblRecheckOk); + lblRecheckOk = sqlite3VdbeMakeLabel(pParse); + if( pIdx->pPartIdxWhere ){ + /* Bypass the recheck if this partial index is not defined + ** for the current row */ + sqlite3VdbeAddOp2(v, OP_IsNull, regIdx, lblRecheckOk); + VdbeCoverage(v); + } + /* Copy the constraint check code from above, except change + ** the constraint-ok jump destination to be the address of + ** the next retest block */ + pOp = sqlite3VdbeGetOp(v, addrConflictCk); + while( nConflictCk>0 && !db->mallocFailed ){ + if( sqlite3OpcodeProperty[pOp->opcode]&OPFLG_JUMP ){ + p2 = lblRecheckOk; + }else{ + p2 = pOp->p2; + } + if( pOp->opcode!=OP_IdxRowid ){ + sqlite3VdbeAddOp4(v, pOp->opcode, pOp->p1, p2, pOp->p3, + pOp->p4.z, pOp->p4type); + sqlite3VdbeChangeP5(v, pOp->p5); + } + nConflictCk--; + pOp++; + } + /* If the retest fails, issue an abort */ sqlite3UniqueConstraint(pParse, OE_Abort, pIdx); + + sqlite3VdbeJumpHere(v, addrBypass); /* Terminate the recheck bypass */ } seenReplace = 1; break; @@ -1867,6 +1951,25 @@ void sqlite3GenerateConstraintChecks( sqlite3VdbeJumpHere(v, ipkBottom); } + /* Recheck all uniqueness constraints after replace triggers have run */ + testcase( regTrigCnt!=0 && nReplaceTrig==0 ); + if( nReplaceTrig ){ + sqlite3VdbeAddOp2(v, OP_IfNot, regTrigCnt, lblRecheckOk);VdbeCoverage(v); + if( !pPk ){ + if( isUpdate ){ + sqlite3VdbeAddOp3(v, OP_Eq, regNewData, addrRecheck, regOldData); + sqlite3VdbeChangeP5(v, SQLITE_NOTNULL); + VdbeCoverage(v); + } + sqlite3VdbeAddOp3(v, OP_NotExists, iDataCur, addrRecheck, regNewData); + VdbeCoverage(v); + sqlite3RowidConstraint(pParse, OE_Abort, pTab); + }else{ + sqlite3VdbeGoto(v, addrRecheck); + } + sqlite3VdbeResolveLabel(v, lblRecheckOk); + } + /* Generate the table record */ if( HasRowid(pTab) ){ int regRec = aRegIdx[ix]; diff --git a/test/insert.test b/test/insert.test index e9bd8d6d7b..b3549289a8 100644 --- a/test/insert.test +++ b/test/insert.test @@ -519,8 +519,29 @@ do_catchsql_test insert-16.6 { } {1 {UNIQUE constraint failed: p1.b}} integrity_check insert-16.7 +# 2019-10-25 ticket c1e19e12046d23fe +do_catchsql_test insert-17.1 { + PRAGMA temp.recursive_triggers = true; + DROP TABLE IF EXISTS t0; + CREATE TABLE t0(aa, bb); + CREATE UNIQUE INDEX t0bb ON t0(bb); + CREATE TRIGGER "r17.1" BEFORE DELETE ON t0 + BEGIN INSERT INTO t0(aa,bb) VALUES(99,1); + END; + INSERT INTO t0(aa,bb) VALUES(10,20); + REPLACE INTO t0(aa,bb) VALUES(30,20); +} {1 {UNIQUE constraint failed: t0.rowid}} +integrity_check insert-17.2 +do_catchsql_test insert-17.3 { + DROP TABLE IF EXISTS t1; + CREATE TABLE t1(a, b UNIQUE, c UNIQUE); + INSERT INTO t1(a,b,c) VALUES(1,1,1),(2,2,2),(3,3,3),(4,4,4); + CREATE TRIGGER "r17.3" AFTER DELETE ON t1 WHEN OLD.c<>3 BEGIN + INSERT INTO t1(rowid,a,b,c) VALUES(100,100,100,3); + END; + REPLACE INTO t1(rowid,a,b,c) VALUES(200,1,2,3); +} {1 {UNIQUE constraint failed: t1.c}} +integrity_check insert-17.4 - - finish_test