From: drh Date: Sat, 14 Apr 2018 20:24:36 +0000 (+0000) Subject: Make sure constraint checks occur in the correct order, even in the X-Git-Tag: version-3.24.0~146^2~15 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=096fd476c1ebb53a8add1f89176bad80eadbd880;p=thirdparty%2Fsqlite.git Make sure constraint checks occur in the correct order, even in the presence of upserts. FossilOrigin-Name: 07fb30c3de7ff396ae2ce8a0d20352b56f17a5db0af99a921c7bfe9bd4018115 --- diff --git a/manifest b/manifest index 504555b7d2..3262a13baa 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C First\scut\sat\slogic\sto\sperform\sDO\sUPDATE\sfor\srowid\stables. -D 2018-04-13T21:55:22.055 +C Make\ssure\sconstraint\schecks\soccur\sin\sthe\scorrect\sorder,\seven\sin\sthe\npresence\sof\supserts. +D 2018-04-14T20:24:36.183 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F Makefile.in 5ce9343cba9c189046f1afe6d2bcc1f68079439febc05267b98aec6ecc752439 @@ -452,7 +452,7 @@ F src/hash.c a12580e143f10301ed5166ea4964ae2853d3905a511d4e0c44497245c7ce1f7a F src/hash.h ab34c5c54a9e9de2e790b24349ba5aab3dbb4fd4 F src/hwtime.h 747c1bbe9df21a92e9c50f3bbec1de841dc5e5da F src/in-operator.md 10cd8f4bcd225a32518407c2fb2484089112fd71 -F src/insert.c 04908e34a009115ce503a0802a872b878185a89e3b43066e11e0f1d176baf987 +F src/insert.c be6ece1650d14f4aa850786f7e08376fc16841423b273759167584c4cd0fda0a F src/legacy.c 134ab3e3fae00a0f67a5187981d6935b24b337bcf0f4b3e5c9fa5763da95bf4e F src/loadext.c f6e4e416a736369f9e80eba609f0acda97148a8b0453784d670c78d3eed2f302 F src/main.c 1648fc7a9bcfdbfd9a9a04af96ff2796c3164b3f3c7e56ed63a3c51cd11d198d @@ -1719,7 +1719,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 6d3017f92bce3e50a91fab2f605e2af8b913b1b374adbfd977299eb042683de8 -R 2377d90753cd9d2cc901deb435ebac46 +P a9080bc8b8c5f3b399eb1819bb5009581f178d85bb2b2cca7bc16a7b81b06863 +R 314a81d3ecdf7de20737770688132742 U drh -Z a8321022a56d460ca2744ec7d9e4710f +Z 5b6df9744700f2b62c7a81bfca411a19 diff --git a/manifest.uuid b/manifest.uuid index e17971758a..4613e6da7e 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -a9080bc8b8c5f3b399eb1819bb5009581f178d85bb2b2cca7bc16a7b81b06863 \ No newline at end of file +07fb30c3de7ff396ae2ce8a0d20352b56f17a5db0af99a921c7bfe9bd4018115 \ No newline at end of file diff --git a/src/insert.c b/src/insert.c index c36d50fb38..d98f127c52 100644 --- a/src/insert.c +++ b/src/insert.c @@ -1157,6 +1157,42 @@ static int checkConstraintUnchanged(Expr *pExpr, int *aiChng, int chngRowid){ return !w.eCode; } +/* +** An instance of the ConstraintAddr object remembers the byte-code addresses +** for sections of the constraint checks that deal with uniqueness constraints +** on the rowid and on the upsert constraint. +** +** This information is passed into checkReorderConstraintChecks() to insert +** some OP_Goto operations so that the rowid and upsert constraints occur +** in the correct order relative to other constraints. +*/ +typedef struct ConstraintAddr ConstraintAddr; +struct ConstraintAddr { + int ipkTop; /* Subroutine for rowid constraint check */ + int upsertTop; /* Label for upsert constraint check subroutine */ + int ipkBtm; /* Return opcode rowid constraint check */ + int upsertBtm; /* upsert constraint returns to this label */ +}; + +/* +** Generate any OP_Goto operations needed to cause constraints to be +** run that haven't already been run. +*/ +static void reorderConstraintChecks(Vdbe *v, ConstraintAddr *p){ + if( p->upsertTop ){ + sqlite3VdbeGoto(v, p->upsertTop); + VdbeComment((v, "call upsert subroutine")); + sqlite3VdbeResolveLabel(v, p->upsertBtm); + p->upsertTop = 0; + } + if( p->ipkTop ){ + sqlite3VdbeGoto(v, p->ipkTop); + VdbeComment((v, "call rowid constraint-check subroutine")); + sqlite3VdbeJumpHere(v, p->ipkBtm); + p->ipkTop = 0; + } +} + /* ** Generate code to do constraint checks prior to an INSERT or an UPDATE ** on table pTab. @@ -1266,10 +1302,11 @@ void sqlite3GenerateConstraintChecks( int addr1; /* Address of jump instruction */ int seenReplace = 0; /* True if REPLACE is used to resolve INT PK conflict */ int nPkField; /* Number of fields in PRIMARY KEY. 1 for ROWID tables */ - int ipkTop = 0; /* Top of the rowid change constraint check */ - int ipkBottom = 0; /* Bottom of the rowid change constraint check */ + ConstraintAddr sAddr;/* Address information for constraint reordering */ + Index *pUpIdx = 0; /* Index to which to apply the upsert */ u8 isUpdate; /* True if this is an UPDATE operation */ u8 bAffinityDone = 0; /* True if the OP_Affinity operation has been run */ + int upsertBypass = 0; /* Address of Goto to bypass upsert subroutine */ isUpdate = regOldData!=0; db = pParse->db; @@ -1277,6 +1314,8 @@ void sqlite3GenerateConstraintChecks( assert( v!=0 ); assert( pTab->pSelect==0 ); /* This table is not a VIEW */ nCol = pTab->nCol; + sAddr.ipkTop = 0; + sAddr.upsertTop = 0; /* pPk is the PRIMARY KEY index for WITHOUT ROWID tables and NULL for ** normal rowid tables. nPkField is the number of key fields in the @@ -1376,6 +1415,53 @@ void sqlite3GenerateConstraintChecks( } #endif /* !defined(SQLITE_OMIT_CHECK) */ + /* UNIQUE and PRIMARY KEY constraints should be handled in the following + ** order: + ** + ** (1) OE_Abort, OE_Fail, OE_Rollback, OE_Ignore + ** (2) OE_Update + ** (3) OE_Replace + ** + ** OE_Fail and OE_Ignore must happen before any changes are made. + ** OE_Update guarantees that only a single row will change, so it + ** must happen before OE_Replace. Technically, OE_Abort and OE_Rollback + ** could happen in any order, but they are grouped up front for + ** convenience. + ** + ** Constraint checking code is generated in this order: + ** (A) The rowid constraint + ** (B) Unique index constraints that do not have OE_Replace as their + ** default conflict resolution strategy + ** (C) Unique index that do use OE_Replace by default. + ** + ** The ordering of (2) and (3) is accomplished by making sure the linked + ** list of indexes attached to a table puts all OE_Replace indexes last + ** in the list. See sqlite3CreateIndex() for where that happens. + */ + + /* If there is an ON CONFLICT clause without a constraint-target + ** (In other words, one of "ON CONFLICT DO NOTHING" or + ** "ON DUPLICATION KEY UPDATE") then change the overrideError to + ** whichever is appropriate. + */ + if( pUpsert ){ + if( pUpsert->pUpsertTarget==0 ){ + if( pUpsert->pUpsertSet==0 ){ + /* An ON CONFLICT DO NOTHING clause, without a constraint-target. + ** Make all unique constraint resolution be OE_Ignore */ + overrideError = OE_Ignore; + pUpsert = 0; + }else{ + /* An ON DUPLICATE KEY UPDATE clause. All unique constraints + ** do upsert processing */ + overrideError = OE_Update; + } + }else if( (pUpIdx = pUpsert->pUpsertIdx)!=0 ){ + sAddr.upsertTop = sqlite3VdbeMakeLabel(v); + sAddr.upsertBtm = sqlite3VdbeMakeLabel(v); + } + } + /* If rowid is changing, make sure the new rowid does not previously ** exist in the table. */ @@ -1400,7 +1486,7 @@ void sqlite3GenerateConstraintChecks( } /* figure out whether or not upsert applies in this case */ - if( pUpsert && (pUpsert->pUpsertTarget==0 || pUpsert->pUpsertIdx==0) ){ + if( pUpsert && pUpsert->pUpsertIdx==0 ){ if( pUpsert->pUpsertSet==0 ){ onError = OE_Ignore; /* DO NOTHING is the same as INSERT OR IGNORE */ }else{ @@ -1413,17 +1499,21 @@ void sqlite3GenerateConstraintChecks( ** to defer the running of the rowid conflict checking until after ** the UNIQUE constraints have run. */ - if( onError==OE_Replace && overrideError!=OE_Replace ){ - for(pIdx=pTab->pIndex; pIdx; pIdx=pIdx->pNext){ - if( pIdx->onError==OE_Ignore || pIdx->onError==OE_Fail ){ - ipkTop = sqlite3VdbeAddOp0(v, OP_Goto); - break; - } - } + assert( OE_Update>OE_Replace ); + assert( OE_Ignore=OE_Replace + && onError!=overrideError + && pTab->pIndex + ){ + sAddr.ipkTop = sqlite3VdbeAddOp0(v, OP_Goto)+1; } /* Check to see if the new rowid already exists in the table. Skip ** the following conflict logic if it does not. */ + VdbeNoopComment((v, "constraint checks for ROWID")); sqlite3VdbeAddOp3(v, OP_NotExists, iDataCur, addrRowidOk, regNewData); VdbeCoverage(v); @@ -1499,9 +1589,9 @@ void sqlite3GenerateConstraintChecks( } } sqlite3VdbeResolveLabel(v, addrRowidOk); - if( ipkTop ){ - ipkBottom = sqlite3VdbeAddOp0(v, OP_Goto); - sqlite3VdbeJumpHere(v, ipkTop); + if( sAddr.ipkTop ){ + sAddr.ipkBtm = sqlite3VdbeAddOp0(v, OP_Goto); + sqlite3VdbeJumpHere(v, sAddr.ipkTop-1); } } @@ -1519,12 +1609,17 @@ void sqlite3GenerateConstraintChecks( int addrUniqueOk; /* Jump here if the UNIQUE constraint is satisfied */ if( aRegIdx[ix]==0 ) continue; /* Skip indices that do not change */ + VdbeNoopComment((v, "constraint checks for %s", pIdx->zName)); if( bAffinityDone==0 ){ sqlite3TableAffinity(v, pTab, regNewData+1); bAffinityDone = 1; } iThisCur = iIdxCur+ix; - addrUniqueOk = sqlite3VdbeMakeLabel(v); + if( pUpIdx==pIdx ){ + addrUniqueOk = sAddr.upsertBtm; + }else{ + addrUniqueOk = sqlite3VdbeMakeLabel(v); + } /* Skip partial indices for which the WHERE clause is not true */ if( pIdx->pPartIdxWhere ){ @@ -1583,14 +1678,20 @@ void sqlite3GenerateConstraintChecks( }else if( onError==OE_Default ){ onError = OE_Abort; } + if( onError==OE_Replace ){ + reorderConstraintChecks(v, &sAddr); + } /* Figure out if the upsert clause applies to this index */ - if( pUpsert && (pUpsert->pUpsertTarget==0 || pUpsert->pUpsertIdx==pIdx) ){ + if( pUpIdx==pIdx ){ if( pUpsert->pUpsertSet==0 ){ onError = OE_Ignore; /* DO NOTHING is the same as INSERT OR IGNORE */ }else{ onError = OE_Update; /* DO UPDATE */ } + upsertBypass = sqlite3VdbeGoto(v, 0); + VdbeComment((v, "Upsert bypass")); + sqlite3VdbeResolveLabel(v, sAddr.upsertTop); } /* Collision detection may be omitted if all of the following are true: @@ -1710,11 +1811,10 @@ void sqlite3GenerateConstraintChecks( sqlite3VdbeResolveLabel(v, addrUniqueOk); sqlite3ExprCachePop(pParse); if( regR!=regIdx ) sqlite3ReleaseTempRange(pParse, regR, nPkField); + if( pUpIdx==pIdx ) sqlite3VdbeJumpHere(v, upsertBypass); + } - if( ipkTop ){ - sqlite3VdbeGoto(v, ipkTop+1); - sqlite3VdbeJumpHere(v, ipkBottom); - } + reorderConstraintChecks(v, &sAddr); *pbMayReplace = seenReplace; VdbeModuleComment((v, "END: GenCnstCks(%d)", seenReplace));