From a7c3b93fa634109dafd4170373d838faf6008d8a Mon Sep 17 00:00:00 2001 From: drh Date: Tue, 7 May 2019 19:13:42 +0000 Subject: [PATCH 1/2] On an INSERT or UPDATE, generate the new table record prior to running foreign key checks, in case the foreign key checks changes datatypes on the registers holding column values. Proposed fix for ticket [e63cbcfd3378afe6980d626]. FossilOrigin-Name: 3f1c8051648a341db4dffad66d3b1f9980d8a2b314cb0ce879cb2a10d1779b84 --- manifest | 17 ++++++++++------- manifest.uuid | 2 +- src/insert.c | 34 +++++++++++++++++++++------------- src/update.c | 7 ++++--- 4 files changed, 36 insertions(+), 24 deletions(-) diff --git a/manifest b/manifest index 2177c1210d..243d45027f 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Strive\sto\sprevent\sharmless\scompiler\swarnings\sin\sGCC\s4.8.5. -D 2019-05-07T17:47:43.636 +C On\san\sINSERT\sor\sUPDATE,\sgenerate\sthe\snew\stable\srecord\sprior\sto\srunning\nforeign\skey\schecks,\sin\scase\sthe\sforeign\skey\schecks\schanges\sdatatypes\son\nthe\sregisters\sholding\scolumn\svalues.\s\sProposed\sfix\sfor\sticket\n[e63cbcfd3378afe6980d626]. +D 2019-05-07T19:13:42.131 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -481,7 +481,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 fc3cf5c371f9a400144e8c2f148ab29cd3f67f7da7eaf47e6a6959f8255fd92c +F src/insert.c 4ffc3aa5d2aed178b501533428a76e150907e92a1e4bf7af4ffbcb0d77e99823 F src/legacy.c d7874bc885906868cd51e6c2156698f2754f02d9eee1bae2d687323c3ca8e5aa F src/loadext.c 22afc33c3a61b4fd80a60a54f1882688371e6bc64685df2696b008fce65a999c F src/main.c 3c3925b0bcb4c45687fd52f54c79e98e379252e1d3393f8b7dcccfa26181b661 @@ -587,7 +587,7 @@ F src/threads.c 4ae07fa022a3dc7c5beb373cf744a85d3c5c6c3c F src/tokenize.c d3615f0cbe4db5949503bf5916f3cd4fa5de855d5b4ef560f3b6dd5629423a1e F src/treeview.c 56724725c62a0d0f408f7c257475dc33309198afee36a1d18be1bc268b09055e F src/trigger.c bb034c08eca111e66a19cda045903a12547c1be2294b5570d794b869d9c44a73 -F src/update.c 0b973357d88092140531e07ff641139c26fb4380b0b9f5ed98c5f7691b4604d1 +F src/update.c fb29c2eb1f52a5009fd0e00f5041fb6ae14a7f9a5048da220e45e804c61b3559 F src/upsert.c 0dd81b40206841814d46942a7337786932475f085716042d0cb2fc7791bf8ca4 F src/utf.c 2f0fac345c7660d5c5bd3df9e9d8d33d4c27f366bcfb09e07443064d751a0507 F src/util.c 5061987401c2e8003177fa30d73196aa036727c8f04bf36a2df0c82b1904a236 @@ -1824,7 +1824,10 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0 -P 82062351a62f4ccc438a2b47a22ee581bd030dec952fecd610f360a25002a5ea -R 4f39e2e7a0d63e22242e3f88eda624cc +P 8b6691f619ed9a56f6aecbd878ebb447c40984f8767508b248494fd9ec68fbaa +R ac9c1b91c829d87b45248e874c9bfe36 +T *branch * tkt-e63cbcfd +T *sym-tkt-e63cbcfd * +T -sym-trunk * U drh -Z 38298ab91e1e4b87a848f20cfed8bc7a +Z 017803aae35252cbb0229aaaad801bc4 diff --git a/manifest.uuid b/manifest.uuid index 974d456905..3b6ab441bb 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -8b6691f619ed9a56f6aecbd878ebb447c40984f8767508b248494fd9ec68fbaa \ No newline at end of file +3f1c8051648a341db4dffad66d3b1f9980d8a2b314cb0ce879cb2a10d1779b84 \ No newline at end of file diff --git a/src/insert.c b/src/insert.c index ee63eeda56..094b98351e 100644 --- a/src/insert.c +++ b/src/insert.c @@ -814,7 +814,7 @@ void sqlite3Insert( int nIdx; nIdx = sqlite3OpenTableAndIndices(pParse, pTab, OP_OpenWrite, 0, -1, 0, &iDataCur, &iIdxCur); - aRegIdx = sqlite3DbMallocRawNN(db, sizeof(int)*(nIdx+1)); + aRegIdx = sqlite3DbMallocRawNN(db, sizeof(int)*(nIdx+2)); if( aRegIdx==0 ){ goto insert_cleanup; } @@ -823,6 +823,7 @@ void sqlite3Insert( aRegIdx[i] = ++pParse->nMem; pParse->nMem += pIdx->nColumn; } + aRegIdx[i] = ++pParse->nMem; /* Register to store the table record */ } #ifndef SQLITE_OMIT_UPSERT if( pUpsert ){ @@ -1226,6 +1227,14 @@ int sqlite3ExprReferencesUpdatedColumn( ** the same as the order of indices on the linked list of indices ** at pTab->pIndex. ** +** (2019-05-07) The generated code also creates a new record for the +** main table, if pTab is a rowid table, and stores that record in the +** register identified by aRegIdx[nIdx] - in other words in the first +** entry of aRegIdx[] past the last index. It is important that the +** record be generated during constraint checks to avoid affinity changes +** to the register content that occur after constraint checks but before +** the new record is inserted. +** ** The caller must have already opened writeable cursors on the main ** table and all applicable indices (that is to say, all indices for which ** aRegIdx[] is not zero). iDataCur is the cursor for the main table when @@ -1845,6 +1854,16 @@ void sqlite3GenerateConstraintChecks( sqlite3VdbeJumpHere(v, ipkBottom); } + /* Generate the table record */ + if( HasRowid(pTab) ){ + int regRec = aRegIdx[ix]; + sqlite3VdbeAddOp3(v, OP_MakeRecord, regNewData+1, pTab->nCol, regRec); + sqlite3SetMakeRecordP5(v, pTab); + if( !bAffinityDone ){ + sqlite3TableAffinity(v, pTab, 0); + } + } + *pbMayReplace = seenReplace; VdbeModuleComment((v, "END: GenCnstCks(%d)", seenReplace)); } @@ -1894,10 +1913,7 @@ void sqlite3CompleteInsertion( Vdbe *v; /* Prepared statements under construction */ Index *pIdx; /* An index being inserted or updated */ u8 pik_flags; /* flag values passed to the btree insert */ - int regData; /* Content registers (after the rowid) */ - int regRec; /* Register holding assembled record for the table */ int i; /* Loop counter */ - u8 bAffinityDone = 0; /* True if OP_Affinity has been run already */ assert( update_flags==0 || update_flags==OPFLAG_ISUPDATE @@ -1909,7 +1925,6 @@ void sqlite3CompleteInsertion( assert( pTab->pSelect==0 ); /* This table is not a VIEW */ for(i=0, pIdx=pTab->pIndex; pIdx; pIdx=pIdx->pNext, i++){ if( aRegIdx[i]==0 ) continue; - bAffinityDone = 1; if( pIdx->pPartIdxWhere ){ sqlite3VdbeAddOp2(v, OP_IsNull, aRegIdx[i], sqlite3VdbeCurrentAddr(v)+2); VdbeCoverage(v); @@ -1937,13 +1952,6 @@ void sqlite3CompleteInsertion( sqlite3VdbeChangeP5(v, pik_flags); } if( !HasRowid(pTab) ) return; - regData = regNewData + 1; - regRec = sqlite3GetTempReg(pParse); - sqlite3VdbeAddOp3(v, OP_MakeRecord, regData, pTab->nCol, regRec); - sqlite3SetMakeRecordP5(v, pTab); - if( !bAffinityDone ){ - sqlite3TableAffinity(v, pTab, 0); - } if( pParse->nested ){ pik_flags = 0; }else{ @@ -1956,7 +1964,7 @@ void sqlite3CompleteInsertion( if( useSeekResult ){ pik_flags |= OPFLAG_USESEEKRESULT; } - sqlite3VdbeAddOp3(v, OP_Insert, iDataCur, regRec, regNewData); + sqlite3VdbeAddOp3(v, OP_Insert, iDataCur, aRegIdx[i], regNewData); if( !pParse->nested ){ sqlite3VdbeAppendP4(v, pTab, P4_TABLE); } diff --git a/src/update.c b/src/update.c index 132837d44f..8bb9f8466a 100644 --- a/src/update.c +++ b/src/update.c @@ -159,7 +159,7 @@ void sqlite3Update( int iDataCur; /* Cursor for the canonical data btree */ int iIdxCur; /* Cursor for the first index */ sqlite3 *db; /* The database structure */ - int *aRegIdx = 0; /* First register in array assigned to each index */ + int *aRegIdx = 0; /* Registers for to each index and the main table */ int *aXRef = 0; /* aXRef[i] is the index in pChanges->a[] of the ** an expression for the i-th column of the table. ** aXRef[i]==-1 if the i-th column is not changed. */ @@ -273,10 +273,10 @@ void sqlite3Update( /* Allocate space for aXRef[], aRegIdx[], and aToOpen[]. ** Initialize aXRef[] and aToOpen[] to their default values. */ - aXRef = sqlite3DbMallocRawNN(db, sizeof(int) * (pTab->nCol+nIdx) + nIdx+2 ); + aXRef = sqlite3DbMallocRawNN(db, sizeof(int) * (pTab->nCol+nIdx+1) + nIdx+2 ); if( aXRef==0 ) goto update_cleanup; aRegIdx = aXRef+pTab->nCol; - aToOpen = (u8*)(aRegIdx+nIdx); + aToOpen = (u8*)(aRegIdx+nIdx+1); memset(aToOpen, 1, nIdx+1); aToOpen[nIdx+1] = 0; for(i=0; inCol; i++) aXRef[i] = -1; @@ -378,6 +378,7 @@ void sqlite3Update( if( reg==0 ) aToOpen[j+1] = 0; aRegIdx[j] = reg; } + aRegIdx[j] = ++pParse->nMem; /* Register storing the table record */ if( bReplace ){ /* If REPLACE conflict resolution might be invoked, open cursors on all ** indexes in case they are needed to delete records. */ From 244edd01d32b4081086982d5f3c5a93b7a2b6f18 Mon Sep 17 00:00:00 2001 From: dan Date: Tue, 7 May 2019 19:21:58 +0000 Subject: [PATCH 2/2] Add test cases for the fix on this branch. FossilOrigin-Name: 2e31abe0ae5937a8ce10179e0ae045ee4c5ed8b7e2622ab41243226c6d3f5425 --- manifest | 17 +++++++---------- manifest.uuid | 2 +- test/fkey8.test | 31 +++++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 11 deletions(-) diff --git a/manifest b/manifest index 243d45027f..1260899fa6 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C On\san\sINSERT\sor\sUPDATE,\sgenerate\sthe\snew\stable\srecord\sprior\sto\srunning\nforeign\skey\schecks,\sin\scase\sthe\sforeign\skey\schecks\schanges\sdatatypes\son\nthe\sregisters\sholding\scolumn\svalues.\s\sProposed\sfix\sfor\sticket\n[e63cbcfd3378afe6980d626]. -D 2019-05-07T19:13:42.131 +C Add\stest\scases\sfor\sthe\sfix\son\sthis\sbranch. +D 2019-05-07T19:21:58.832 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -859,7 +859,7 @@ F test/fkey4.test 86446017011273aad8f9a99c1a65019e7bd9ca9d F test/fkey5.test 24dd28eb3d9f1b5a174f47e9899ace5facb08373a4223593c8c631e6cf9f7d5a F test/fkey6.test d078a1e323a740062bed38df32b8a736fd320dc0 F test/fkey7.test 24076d43d3449f12f25503909ca4bfb5fc5fefd5af1f930723a496343eb28454 -F test/fkey8.test 863c6d84f0d289fd2c1a1c293abb9803f77efd35211d9012c0986c8f6ccf5d5a +F test/fkey8.test 1d44df25d3b9cba72db4b4324201daf6ae1fc8a85cb68146bd6669a977d8867d F test/fkey_malloc.test 594a7ea1fbab553c036c70813cd8bd9407d63749 F test/fordelete.test eb93a2f34137bb87bdab88fcab06c0bd92719aff F test/format4.test 1f0cac8ff3895e9359ed87e41aaabee982a812eb @@ -1824,10 +1824,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 8b6691f619ed9a56f6aecbd878ebb447c40984f8767508b248494fd9ec68fbaa -R ac9c1b91c829d87b45248e874c9bfe36 -T *branch * tkt-e63cbcfd -T *sym-tkt-e63cbcfd * -T -sym-trunk * -U drh -Z 017803aae35252cbb0229aaaad801bc4 +P 3f1c8051648a341db4dffad66d3b1f9980d8a2b314cb0ce879cb2a10d1779b84 +R 2d97d9191e7764860c32bedf82b06a8b +U dan +Z 37627195381a6f7ef79ed6ec381c9c3a diff --git a/manifest.uuid b/manifest.uuid index 3b6ab441bb..35ac028a5a 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -3f1c8051648a341db4dffad66d3b1f9980d8a2b314cb0ce879cb2a10d1779b84 \ No newline at end of file +2e31abe0ae5937a8ce10179e0ae045ee4c5ed8b7e2622ab41243226c6d3f5425 \ No newline at end of file diff --git a/test/fkey8.test b/test/fkey8.test index f38e835aef..42f997f06f 100644 --- a/test/fkey8.test +++ b/test/fkey8.test @@ -197,5 +197,36 @@ do_execsql_test 4.2 { INSERT OR REPLACE INTO t1 VALUES(20000, 20000); } +#------------------------------------------------------------------------- +reset_db +do_execsql_test 5.0 { + PRAGMA foreign_keys = true; + CREATE TABLE parent( + p TEXT PRIMARY KEY + ); + CREATE TABLE child( + c INTEGER UNIQUE, + FOREIGN KEY(c) REFERENCES parent(p) DEFERRABLE INITIALLY DEFERRED + ); + BEGIN; + INSERT INTO child VALUES(123); + INSERT INTO parent VALUES('123'); + COMMIT; +} +do_execsql_test 5.1 { + PRAGMA integrity_check; +} {ok} + +do_execsql_test 5.2 { + INSERT INTO parent VALUES(1200); + BEGIN; + INSERT INTO child VALUES(456); + UPDATE parent SET p = '456' WHERE p=1200; + COMMIT; +} +do_execsql_test 5.3 { + PRAGMA integrity_check; +} {ok} + finish_test