From: dan Date: Sat, 18 Jul 2026 19:31:56 +0000 (+0000) Subject: After a DELETE trigger is fired by REPLACE conflict handling, retest all UNIQUE const... X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=c1c108fd7119f4f8f34317768c5664494886754d;p=thirdparty%2Fsqlite.git After a DELETE trigger is fired by REPLACE conflict handling, retest all UNIQUE constraints to ensure that the row being inserted or updated does not conflict with one written by the DELETE trigger. Abort the statement if it does. Possible fix for forum post [forum:/info/2026-07-18T03:35:48Z | 2026-07-18T03:35:48Z]. FossilOrigin-Name: e2ab286bb713e1358ae9a1d50dc894362d0ea6f1a2e8467c587f42f0e483dd3a --- diff --git a/manifest b/manifest index 15ec56e329..9a17e98c05 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Enhancements\sto\snew\stest\scases,\sfor\simproved\sreadability. -D 2026-07-18T09:04:36.385 +C After\sa\sDELETE\strigger\sis\sfired\sby\sREPLACE\sconflict\shandling,\sretest\sall\sUNIQUE\sconstraints\sto\sensure\sthat\sthe\srow\sbeing\sinserted\sor\supdated\sdoes\snot\sconflict\swith\sone\swritten\sby\sthe\sDELETE\strigger.\sAbort\sthe\sstatement\sif\sit\sdoes.\sPossible\sfix\sfor\sforum\spost\s[forum:/info/2026-07-18T03:35:48Z\s|\s2026-07-18T03:35:48Z]. +D 2026-07-18T19:31:56.512 F .fossil-settings/binary-glob 61195414528fb3ea9693577e1980230d78a1f8b0a54c78cf1b9b24d0a409ed6a x F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea @@ -702,7 +702,7 @@ F src/hash.c 03c8c0f4be9e8bcb6de65aa26d34a61d48a9430747084a69f9469fbb00ea52ca F src/hash.h 46b92795a95bfefb210f52f0c316e9d7cdbcdd7e7fcfb0d8be796d3a5767cddf F src/hwtime.h 5cb15147c8583d0fc4748e1c12ea6f38c9deaeefa147a4d8d379fd9bc81fee9a F src/in-operator.md 10cd8f4bcd225a32518407c2fb2484089112fd71 -F src/insert.c 71776cca5bf05b05b1427fa1dc1b0d391e7f3805abaf9b6f3c24ec7350040838 +F src/insert.c 64c22744bce3b1e9630288043c1967a60b4f02c8fc7707fdb10d13c1ec30d297 F src/json.c e4af4d799e4e7630c633b3b356d65e92d1781e42343ab77b8ef747c932762012 F src/legacy.c d7874bc885906868cd51e6c2156698f2754f02d9eee1bae2d687323c3ca8e5aa F src/loadext.c 5cd4cd7ec6e1a46416806899086363fbf95eecd3bfb9a388b436c9a77243c902 @@ -1947,7 +1947,7 @@ F test/upfrom2.test 66f3ebf721b3cebd922faee5c386bf244f816d416b57c000753ff51af623 F test/upfrom3.test 6130f24ebf97f5ea865e5d2a14a2d543fe5428a62e87cc60f62d875e45c1f5f0 F test/upfrom4.test 78f742a6577c91a7a55c64edb8811004e7c6aa99b8d57b2320f70a918c357807 F test/upfromfault.test 3a10075a0043f0c4fad6614b2c371f88a8ba5a4acab68b907438413865d6a8d6 -F test/upsert1.test 107290c1f7c4d504fa8190e479841ad29403af529f1cefef5c1fd501a6a584e1 +F test/upsert1.test d29de66c2a034a2ed83a3e26e693e9d02bd3b6e0e67dcde62631c6e15023ca79 F test/upsert2.test 720e94d09f7362a282bc69b3c6b83d51daeaaf0440eb4920a08b86518b8c7496 F test/upsert3.test 88d7d590a1948a9cb6eac1b54b0642f67a9f35a1fc0f19b200e97d5d39e3179c F test/upsert4.test 25d2a1da92f149331ae0c51ca6e3eee78189577585eab92de149900d62994fa5 @@ -2215,8 +2215,8 @@ F tool/warnings-clang.sh bbf6a1e685e534c92ec2bfba5b1745f34fb6f0bc2a362850723a9ee F tool/warnings.sh a554d13f6e5cf3760f041b87939e3d616ec6961859c3245e8ef701d1eafc2ca2 F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f F tool/winmain.c 00c8fb88e365c9017db14c73d3c78af62194d9644feaf60e220ab0f411f3604c -P 49b67b7da6bceae3ee87153480e676f53a472b0f6e0ed8d1866ed819e8f64fa1 -R a1baa1ad016fb074a57c9a0c952a57d3 -U drh -Z a3259deb28c29afa079e0e7d4cb177b8 +P 053e0d1f011d9ff4d57a6e33adf8de9a7a5550a3c5f6a6d4c511f75b85a09a99 +R 732f1b3492a9559cb0f57b5c2b2574f5 +U dan +Z d92037b0ac0aaa02a191794baee1be8a # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 6233bce612..527b644fb5 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -053e0d1f011d9ff4d57a6e33adf8de9a7a5550a3c5f6a6d4c511f75b85a09a99 +e2ab286bb713e1358ae9a1d50dc894362d0ea6f1a2e8467c587f42f0e483dd3a diff --git a/src/insert.c b/src/insert.c index 15a9a5fe89..a981968973 100644 --- a/src/insert.c +++ b/src/insert.c @@ -2230,6 +2230,22 @@ void sqlite3GenerateConstraintChecks( pTrigger = 0; regTrigCnt = sqlite3FkRequired(pParse, pTab, 0, 0); } + if( regTrigCnt ){ + /* At this point regTrigCnt is non-zero if there are DELETE triggers + ** or FK triggers. But we only care about these things if there is + ** a chance that a row will be deleted by an ON CONFLICT REPLACE + ** constraint. So zero regTrigCnt if no such constraint can be found. */ + if( overrideError!=OE_Replace ){ + if( overrideError!=OE_Default ){ + regTrigCnt = 0; + }else if( pkChng==0 || pPk || pTab->keyConf!=OE_Replace ){ + for(pIdx=pTab->pIndex; pIdx; pIdx=pIdx->pNext){ + if( pIdx->onError==OE_Replace ) break; + } + if( pIdx==0 ) regTrigCnt = 0; + } + } + } if( regTrigCnt ){ /* Replace triggers might exist. Allocate the counter and ** initialize it to zero. */ @@ -2405,6 +2421,7 @@ void sqlite3GenerateConstraintChecks( 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 */ + int nConflictCk; /* Number of opcodes in conflict check logic */ if( aRegIdx[ix]==0 ) continue; /* Skip indices that do not change */ if( pUpsert ){ @@ -2581,6 +2598,11 @@ void sqlite3GenerateConstraintChecks( } } + nConflictCk = sqlite3VdbeCurrentAddr(v) - addrConflictCk; + assert( nConflictCk>0 || db->mallocFailed ); + testcase( nConflictCk<=0 ); + testcase( nConflictCk>1 ); + /* Generate code that executes if the new index entry is not unique */ assert( onError==OE_Rollback || onError==OE_Abort || onError==OE_Fail || onError==OE_Ignore || onError==OE_Replace || onError==OE_Update ); @@ -2606,13 +2628,7 @@ void sqlite3GenerateConstraintChecks( break; } default: { - int nConflictCk; /* Number of opcodes in conflict check logic */ - assert( onError==OE_Replace ); - nConflictCk = sqlite3VdbeCurrentAddr(v) - addrConflictCk; - assert( nConflictCk>0 || db->mallocFailed ); - testcase( nConflictCk<=0 ); - testcase( nConflictCk>1 ); if( regTrigCnt ){ sqlite3MultiWrite(pParse); nReplaceTrig++; @@ -2626,57 +2642,57 @@ void sqlite3GenerateConstraintChecks( if( pTrigger && isUpdate ){ sqlite3VdbeAddOp1(v, OP_CursorUnlock, iDataCur); } - if( regTrigCnt ){ - int addrBypass; /* Jump destination to bypass recheck logic */ + seenReplace = 1; + break; + } + } + if( regTrigCnt ){ + 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-1, 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 */ - while( nConflictCk>0 ){ - VdbeOp x; /* Conflict check opcode to copy */ - /* The sqlite3VdbeAddOp4() call might reallocate the opcode array. - ** Hence, make a complete copy of the opcode, rather than using - ** a pointer to the opcode. */ - x = *sqlite3VdbeGetOp(v, addrConflictCk); - if( x.opcode!=OP_IdxRowid ){ - int p2; /* New P2 value for copied conflict check opcode */ - const char *zP4; - if( sqlite3OpcodeProperty[x.opcode]&OPFLG_JUMP ){ - p2 = lblRecheckOk; - }else{ - p2 = x.p2; - } - zP4 = x.p4type==P4_INT32 ? SQLITE_INT_TO_PTR(x.p4.i) : x.p4.z; - sqlite3VdbeAddOp4(v, x.opcode, x.p1, p2, x.p3, zP4, x.p4type); - sqlite3VdbeChangeP5(v, x.p5); - VdbeCoverageIf(v, p2!=x.p2); - } - nConflictCk--; - addrConflictCk++; - } - /* If the retest fails, issue an abort */ - sqlite3UniqueConstraint(pParse, OE_Abort, pIdx); + sqlite3VdbeAddOp2(v, OP_AddImm, regTrigCnt, 1); /* incr trigger cnt */ + addrBypass = sqlite3VdbeAddOp0(v, OP_Goto); /* Bypass recheck */ + VdbeComment((v, "bypass recheck")); - sqlite3VdbeJumpHere(v, addrBypass); /* Terminate the recheck bypass */ + /* 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-1, 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 */ + while( nConflictCk>0 ){ + VdbeOp x; /* Conflict check opcode to copy */ + /* The sqlite3VdbeAddOp4() call might reallocate the opcode array. + ** Hence, make a complete copy of the opcode, rather than using + ** a pointer to the opcode. */ + x = *sqlite3VdbeGetOp(v, addrConflictCk); + if( x.opcode!=OP_IdxRowid ){ + int p2; /* New P2 value for copied conflict check opcode */ + const char *zP4; + if( sqlite3OpcodeProperty[x.opcode]&OPFLG_JUMP ){ + p2 = lblRecheckOk; + }else{ + p2 = x.p2; + } + zP4 = x.p4type==P4_INT32 ? SQLITE_INT_TO_PTR(x.p4.i) : x.p4.z; + sqlite3VdbeAddOp4(v, x.opcode, x.p1, p2, x.p3, zP4, x.p4type); + sqlite3VdbeChangeP5(v, x.p5); + VdbeCoverageIf(v, p2!=x.p2); } - seenReplace = 1; - break; + nConflictCk--; + addrConflictCk++; } + /* If the retest fails, issue an abort */ + sqlite3UniqueConstraint(pParse, OE_Abort, pIdx); + + sqlite3VdbeJumpHere(v, addrBypass); /* Terminate the recheck bypass */ } sqlite3VdbeResolveLabel(v, addrUniqueOk); if( regR!=regIdx ) sqlite3ReleaseTempRange(pParse, regR, nPkField); @@ -2715,8 +2731,8 @@ void sqlite3GenerateConstraintChecks( }else{ sqlite3VdbeGoto(v, addrRecheck); } - sqlite3VdbeResolveLabel(v, lblRecheckOk); } + if( regTrigCnt ) sqlite3VdbeResolveLabel(v, lblRecheckOk); /* Generate the table record */ if( HasRowid(pTab) ){ diff --git a/test/upsert1.test b/test/upsert1.test index 40b9d5d0f8..0b0287e9ae 100644 --- a/test/upsert1.test +++ b/test/upsert1.test @@ -405,4 +405,32 @@ do_execsql_test upsert1-1420 { SELECT * FROM t2 ORDER BY a; } {2 20 200 4 40 400 99 30 999} +# This is a slightly simpler version of the test above (1417-1420). Before +# the bug was fixed, this would: +# +# 1) Checks there are no instances of 'HELLO' in the index on t1(b), +# 2) Deletes the row from t1 as a result of the REPLACE on the PK, +# 3) Fires the trigger, inserting ('HELLO', 200) into index t1(b). +# 4) Finishes the insert without rechecking t1(b), inserting +# ('HELLO', 100). Corruption. +# +reset_db +do_execsql_test upsert-1421 { + PRAGMA recursive_triggers = ON; + + CREATE TABLE t1( + a PRIMARY KEY ON CONFLICT REPLACE, + b UNIQUE + ); + INSERT INTO t1 VALUES(100, 'abcde'); + + CREATE TRIGGER tr1 AFTER DELETE ON t1 BEGIN + INSERT INTO t1 VALUES(200, 'HELLO'); + END; + + INSERT INTO t1 VALUES(100, 'HELLO'); + + SELECT * FROM t1; +} + finish_test