From fb2213e1ff30891cc109015442f9dee2e3e2fbd5 Mon Sep 17 00:00:00 2001 From: drh Date: Fri, 20 Apr 2018 15:56:24 +0000 Subject: [PATCH] Avoid unnecessary cursor seeks during upsert processing. FossilOrigin-Name: 7c4b6d5475092a3e205f01a6972366e27a404568e8e7ba327f2feefac2ce2c7c --- manifest | 19 +++-- manifest.uuid | 2 +- src/update.c | 223 ++++++++++++++++++++++++++++---------------------- src/upsert.c | 69 ++++++---------- 4 files changed, 160 insertions(+), 153 deletions(-) diff --git a/manifest b/manifest index 8aa4cda223..3f45c797e9 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Add\stest\scases\sfor\sUPSERT.\sAnd\sa\sfix\sfor\sa\s"REPLACE\sINTO\s...\sON\sCONFLICT"\nstatement\swhere\sthe\snew\srow\sconflicts\swith\sboth\sthe\sIPK\sand\sthe\sON\sCONFLICT\nindexes. -D 2018-04-20T15:34:08.067 +C Avoid\sunnecessary\scursor\sseeks\sduring\supsert\sprocessing. +D 2018-04-20T15:56:24.816 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F Makefile.in 5ce9343cba9c189046f1afe6d2bcc1f68079439febc05267b98aec6ecc752439 @@ -557,8 +557,8 @@ F src/threads.c 4ae07fa022a3dc7c5beb373cf744a85d3c5c6c3c F src/tokenize.c 5b0c661a85f783d35b9883830736eeb63be4aefc4f6b7d9cd081d48782c041e2 F src/treeview.c 14d5d1254702ec96876aa52642cb31548612384134970409fae333b25b39d6bb F src/trigger.c 4ace6d1d5ba9a89822deb287317f33c810440526eafe185c2d8a48c31df1e995 -F src/update.c 86e6563430a05d9df439cb0253ade0616e3320ef38fa0ed20fb1634e2d9aca0a -F src/upsert.c 71ebb84f330b0d72675c694303a3801633ff4079af9a40569f7e528219484969 +F src/update.c 2f460e57440cd673bb375b5601029d736f3d604cad2b476e31b1f63b10356bce +F src/upsert.c 4f1d04b8cbae727c066cb2203dcefb6c4a4e3c54ba6345e5ec0b515e48381299 F src/utf.c 810fbfebe12359f10bc2a011520a6e10879ab2a163bcb26c74768eab82ea62a5 F src/util.c d9eb0a6c4aae1b00a7369eadd7ca0bbe946cb4c953b6751aa20d357c2f482157 F src/vacuum.c 762ee9bbf8733d87d8cd06f58d950e881982e416f8c767334a40ffd341b6bff5 @@ -1724,7 +1724,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 c37f39d18d41ae5ba6c4561d87cbbf71f3b6896b86cc5cff9cdf046b02dc521a -R ae514c5e5a987302d42c4363153aaf33 -U dan -Z a0358cccdac92456839b19732d4467cc +P d8eb9f8d9b61400c7e12f01ef5c233257b03532221f7c7a8386f7ac2db439626 +R 75119827b876db768b1909c51e8a9eca +T *branch * upsert-opt2 +T *sym-upsert-opt2 * +T -sym-trunk * +U drh +Z 388a812d3bddf28e9c1a7fb5f5cfd19e diff --git a/manifest.uuid b/manifest.uuid index 67904869cc..d25963f495 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -d8eb9f8d9b61400c7e12f01ef5c233257b03532221f7c7a8386f7ac2db439626 \ No newline at end of file +7c4b6d5475092a3e205f01a6972366e27a404568e8e7ba327f2feefac2ce2c7c \ No newline at end of file diff --git a/src/update.c b/src/update.c index 92164b6266..7614838017 100644 --- a/src/update.c +++ b/src/update.c @@ -212,6 +212,7 @@ void sqlite3Update( pParse->nTab++; } if( pUpsert ){ + /* On an UPSERT, reuse the same cursors already opened by INSERT */ iDataCur = pUpsert->iDataCur; iIdxCur = pUpsert->iIdxCur; pParse->nTab = iBaseCur; @@ -389,64 +390,85 @@ void sqlite3Update( } #endif - /* Initialize the count of updated rows */ - if( (db->flags&SQLITE_CountRows)!=0 - && !pParse->pTriggerTab - && !pParse->nested - && !pUpsert - ){ - regRowCount = ++pParse->nMem; - sqlite3VdbeAddOp2(v, OP_Integer, 0, regRowCount); - } + /* Jump to labelBreak to abandon further processing of this UPDATE */ + labelBreak = sqlite3VdbeMakeLabel(v); - if( HasRowid(pTab) ){ - sqlite3VdbeAddOp3(v, OP_Null, 0, regRowSet, regOldRowid); + if( pUpsert ){ + /* If this is an UPSERT, then all cursors have already been opened by + ** the outer INSERT and the data cursor should be pointing at the row + ** that is to be updated. So bypass the code that searches for the + ** row(s) to be updated. + */ + pWInfo = 0; + eOnePass = ONEPASS_SINGLE; + labelContinue = labelBreak; + if( !HasRowid(pTab) ){ + nPk = pPk->nKeyCol; + iPk = pParse->nMem+1; + pParse->nMem += nPk; + regKey = ++pParse->nMem; + } + sqlite3ExprIfFalse(pParse, pWhere, labelBreak, SQLITE_JUMPIFNULL); }else{ - assert( pPk!=0 ); - nPk = pPk->nKeyCol; - iPk = pParse->nMem+1; - pParse->nMem += nPk; - regKey = ++pParse->nMem; - iEph = pParse->nTab++; - - sqlite3VdbeAddOp3(v, OP_Null, 0, iPk, iPk+nPk-1); - addrOpen = sqlite3VdbeAddOp2(v, OP_OpenEphemeral, iEph, nPk); - sqlite3VdbeSetP4KeyInfo(pParse, pPk); - } - - /* Begin the database scan. - ** - ** Do not consider a single-pass strategy for a multi-row update if - ** there are any triggers or foreign keys to process, or rows may - ** be deleted as a result of REPLACE conflict handling. Any of these - ** things might disturb a cursor being used to scan through the table - ** or index, causing a single-pass approach to malfunction. */ - flags = WHERE_ONEPASS_DESIRED|WHERE_SEEK_UNIQ_TABLE; - if( !pParse->nested && !pTrigger && !hasFK && !chngKey && !bReplace ){ - flags |= WHERE_ONEPASS_MULTIROW; - } - pWInfo = sqlite3WhereBegin(pParse, pTabList, pWhere, 0, 0, flags, iIdxCur); - if( pWInfo==0 ) goto update_cleanup; - - /* A one-pass strategy that might update more than one row may not - ** be used if any column of the index used for the scan is being - ** updated. Otherwise, if there is an index on "b", statements like - ** the following could create an infinite loop: - ** - ** UPDATE t1 SET b=b+1 WHERE b>? - ** - ** Fall back to ONEPASS_OFF if where.c has selected a ONEPASS_MULTI - ** strategy that uses an index for which one or more columns are being - ** updated. */ - eOnePass = sqlite3WhereOkOnePass(pWInfo, aiCurOnePass); - if( eOnePass==ONEPASS_MULTI ){ - int iCur = aiCurOnePass[1]; - if( iCur>=0 && iCur!=iDataCur && aToOpen[iCur-iBaseCur] ){ - eOnePass = ONEPASS_OFF; + /* Not an UPSERT. Normal processing. Begin by + ** initialize the count of updated rows */ + if( (db->flags&SQLITE_CountRows)!=0 + && !pParse->pTriggerTab + && !pParse->nested + ){ + regRowCount = ++pParse->nMem; + sqlite3VdbeAddOp2(v, OP_Integer, 0, regRowCount); + } + + if( HasRowid(pTab) ){ + sqlite3VdbeAddOp3(v, OP_Null, 0, regRowSet, regOldRowid); + }else{ + assert( pPk!=0 ); + nPk = pPk->nKeyCol; + iPk = pParse->nMem+1; + pParse->nMem += nPk; + regKey = ++pParse->nMem; + iEph = pParse->nTab++; + + sqlite3VdbeAddOp3(v, OP_Null, 0, iPk, iPk+nPk-1); + addrOpen = sqlite3VdbeAddOp2(v, OP_OpenEphemeral, iEph, nPk); + sqlite3VdbeSetP4KeyInfo(pParse, pPk); } - assert( iCur!=iDataCur || !HasRowid(pTab) ); - } + /* Begin the database scan. + ** + ** Do not consider a single-pass strategy for a multi-row update if + ** there are any triggers or foreign keys to process, or rows may + ** be deleted as a result of REPLACE conflict handling. Any of these + ** things might disturb a cursor being used to scan through the table + ** or index, causing a single-pass approach to malfunction. */ + flags = WHERE_ONEPASS_DESIRED|WHERE_SEEK_UNIQ_TABLE; + if( !pParse->nested && !pTrigger && !hasFK && !chngKey && !bReplace ){ + flags |= WHERE_ONEPASS_MULTIROW; + } + pWInfo = sqlite3WhereBegin(pParse, pTabList, pWhere, 0, 0, flags, iIdxCur); + if( pWInfo==0 ) goto update_cleanup; + + /* A one-pass strategy that might update more than one row may not + ** be used if any column of the index used for the scan is being + ** updated. Otherwise, if there is an index on "b", statements like + ** the following could create an infinite loop: + ** + ** UPDATE t1 SET b=b+1 WHERE b>? + ** + ** Fall back to ONEPASS_OFF if where.c has selected a ONEPASS_MULTI + ** strategy that uses an index for which one or more columns are being + ** updated. */ + eOnePass = sqlite3WhereOkOnePass(pWInfo, aiCurOnePass); + if( eOnePass==ONEPASS_MULTI ){ + int iCur = aiCurOnePass[1]; + if( iCur>=0 && iCur!=iDataCur && aToOpen[iCur-iBaseCur] ){ + eOnePass = ONEPASS_OFF; + } + assert( iCur!=iDataCur || !HasRowid(pTab) ); + } + } + if( HasRowid(pTab) ){ /* Read the rowid of the current row of the WHERE scan. In ONEPASS_OFF ** mode, write the rowid into the FIFO. In either of the one-pass modes, @@ -466,7 +488,7 @@ void sqlite3Update( sqlite3ExprCodeGetColumnOfTable(v, pTab, iDataCur,pPk->aiColumn[i],iPk+i); } if( eOnePass ){ - sqlite3VdbeChangeToNoop(v, addrOpen); + if( addrOpen ) sqlite3VdbeChangeToNoop(v, addrOpen); nKey = nPk; regKey = iPk; }else{ @@ -476,55 +498,56 @@ void sqlite3Update( } } - if( eOnePass!=ONEPASS_MULTI ){ - sqlite3WhereEnd(pWInfo); - } - - labelBreak = sqlite3VdbeMakeLabel(v); - if( !isView && pUpsert==0 ){ - int addrOnce = 0; - - /* Open every index that needs updating. */ - if( eOnePass!=ONEPASS_OFF ){ - if( aiCurOnePass[0]>=0 ) aToOpen[aiCurOnePass[0]-iBaseCur] = 0; - if( aiCurOnePass[1]>=0 ) aToOpen[aiCurOnePass[1]-iBaseCur] = 0; + if( pUpsert==0 ){ + if( eOnePass!=ONEPASS_MULTI ){ + sqlite3WhereEnd(pWInfo); } - - if( eOnePass==ONEPASS_MULTI && (nIdx-(aiCurOnePass[1]>=0))>0 ){ - addrOnce = sqlite3VdbeAddOp0(v, OP_Once); VdbeCoverage(v); - } - sqlite3OpenTableAndIndices(pParse, pTab, OP_OpenWrite, 0, iBaseCur, aToOpen, - 0, 0); - if( addrOnce ) sqlite3VdbeJumpHere(v, addrOnce); - } - - /* Top of the update loop */ - if( eOnePass!=ONEPASS_OFF ){ - if( !isView && aiCurOnePass[0]!=iDataCur && aiCurOnePass[1]!=iDataCur ){ - assert( pPk ); - sqlite3VdbeAddOp4Int(v, OP_NotFound, iDataCur, labelBreak, regKey, nKey); - VdbeCoverageNeverTaken(v); + + if( !isView ){ + int addrOnce = 0; + + /* Open every index that needs updating. */ + if( eOnePass!=ONEPASS_OFF ){ + if( aiCurOnePass[0]>=0 ) aToOpen[aiCurOnePass[0]-iBaseCur] = 0; + if( aiCurOnePass[1]>=0 ) aToOpen[aiCurOnePass[1]-iBaseCur] = 0; + } + + if( eOnePass==ONEPASS_MULTI && (nIdx-(aiCurOnePass[1]>=0))>0 ){ + addrOnce = sqlite3VdbeAddOp0(v, OP_Once); VdbeCoverage(v); + } + sqlite3OpenTableAndIndices(pParse, pTab, OP_OpenWrite, 0, iBaseCur, + aToOpen, 0, 0); + if( addrOnce ) sqlite3VdbeJumpHere(v, addrOnce); } - if( eOnePass==ONEPASS_SINGLE ){ - labelContinue = labelBreak; - }else{ + + /* Top of the update loop */ + if( eOnePass!=ONEPASS_OFF ){ + if( !isView && aiCurOnePass[0]!=iDataCur && aiCurOnePass[1]!=iDataCur ){ + assert( pPk ); + sqlite3VdbeAddOp4Int(v, OP_NotFound, iDataCur, labelBreak, regKey,nKey); + VdbeCoverageNeverTaken(v); + } + if( eOnePass==ONEPASS_SINGLE ){ + labelContinue = labelBreak; + }else{ + labelContinue = sqlite3VdbeMakeLabel(v); + } + sqlite3VdbeAddOp2(v, OP_IsNull, pPk ? regKey : regOldRowid, labelBreak); + VdbeCoverageIf(v, pPk==0); + VdbeCoverageIf(v, pPk!=0); + }else if( pPk ){ labelContinue = sqlite3VdbeMakeLabel(v); + sqlite3VdbeAddOp2(v, OP_Rewind, iEph, labelBreak); VdbeCoverage(v); + addrTop = sqlite3VdbeAddOp2(v, OP_RowData, iEph, regKey); + sqlite3VdbeAddOp4Int(v, OP_NotFound, iDataCur, labelContinue, regKey, 0); + VdbeCoverage(v); + }else{ + labelContinue = sqlite3VdbeAddOp3(v, OP_RowSetRead, regRowSet,labelBreak, + regOldRowid); + VdbeCoverage(v); + sqlite3VdbeAddOp3(v, OP_NotExists, iDataCur, labelContinue, regOldRowid); + VdbeCoverage(v); } - sqlite3VdbeAddOp2(v, OP_IsNull, pPk ? regKey : regOldRowid, labelBreak); - VdbeCoverageIf(v, pPk==0); - VdbeCoverageIf(v, pPk!=0); - }else if( pPk ){ - labelContinue = sqlite3VdbeMakeLabel(v); - sqlite3VdbeAddOp2(v, OP_Rewind, iEph, labelBreak); VdbeCoverage(v); - addrTop = sqlite3VdbeAddOp2(v, OP_RowData, iEph, regKey); - sqlite3VdbeAddOp4Int(v, OP_NotFound, iDataCur, labelContinue, regKey, 0); - VdbeCoverage(v); - }else{ - labelContinue = sqlite3VdbeAddOp3(v, OP_RowSetRead, regRowSet, labelBreak, - regOldRowid); - VdbeCoverage(v); - sqlite3VdbeAddOp3(v, OP_NotExists, iDataCur, labelContinue, regOldRowid); - VdbeCoverage(v); } /* If the rowid value will change, set register regNewRowid to diff --git a/src/upsert.c b/src/upsert.c index 80c0056c0b..568dffb53d 100644 --- a/src/upsert.c +++ b/src/upsert.c @@ -203,62 +203,43 @@ void sqlite3UpsertDoUpdate( ){ Vdbe *v = pParse->pVdbe; sqlite3 *db = pParse->db; - int regKey; /* Register(s) containing the key */ - Expr *pWhere; /* Where clause for the UPDATE */ - Expr *pE1, *pE2; SrcList *pSrc; /* FROM clause for the UPDATE */ + int iDataCur = pUpsert->iDataCur; assert( v!=0 ); VdbeNoopComment((v, "Begin DO UPDATE of UPSERT")); - pWhere = sqlite3ExprDup(db, pUpsert->pUpsertWhere, 0); - if( pIdx==0 || HasRowid(pTab) ){ - /* We are dealing with an IPK */ - regKey = ++pParse->nMem; - if( pIdx ){ - sqlite3VdbeAddOp2(v, OP_IdxRowid, iCur, regKey); + if( pIdx && iCur!=iDataCur ){ + if( HasRowid(pTab) ){ + int regRowid = sqlite3GetTempReg(pParse); + sqlite3VdbeAddOp2(v, OP_IdxRowid, iCur, regRowid); + sqlite3VdbeAddOp3(v, OP_SeekRowid, iDataCur, 0, regRowid); + VdbeCoverage(v); + sqlite3ReleaseTempReg(pParse, regRowid); }else{ - sqlite3VdbeAddOp2(v, OP_Rowid, iCur, regKey); - } - pE1 = sqlite3ExprAlloc(db, TK_COLUMN, 0, 0); - if( pE1 ){ - pE1->pTab = pTab; - pE1->iTable = pUpsert->iDataCur; - pE1->iColumn = -1; - } - pE2 = sqlite3ExprAlloc(db, TK_REGISTER, 0, 0); - if( pE2 ){ - pE2->iTable = regKey; - pE2->affinity = SQLITE_AFF_INTEGER; - } - pWhere = sqlite3ExprAnd(db,pWhere,sqlite3PExpr(pParse, TK_EQ, pE1, pE2)); - }else{ - /* a WITHOUT ROWID table */ - int i, j; - for(i=0; inKeyCol; i++){ - regKey = ++pParse->nMem; - sqlite3VdbeAddOp3(v, OP_Column, iCur, i, regKey); - j = pIdx->aiColumn[i]; - VdbeComment((v, "%s", pTab->aCol[j].zName)); - pE1 = sqlite3ExprAlloc(db, TK_COLUMN, 0, 0); - if( pE1 ){ - pE1->pTab = pTab; - pE1->iTable = pUpsert->iDataCur; - pE1->iColumn = j; - } - pE2 = sqlite3ExprAlloc(db, TK_REGISTER, 0, 0); - if( pE2 ){ - pE2->iTable = regKey; - pE2->affinity = pTab->zColAff[j]; + Index *pPk = sqlite3PrimaryKeyIndex(pTab); + int nPk = pPk->nKeyCol; + int iPk = pParse->nMem+1; + int i; + pParse->nMem += nPk; + for(i=0; iaiColumn[i]>=0 ); + k = sqlite3ColumnOfIndex(pIdx, pPk->aiColumn[i]); + sqlite3VdbeAddOp3(v, OP_Column, iCur, k, iPk+i); } - pWhere = sqlite3ExprAnd(db,pWhere,sqlite3PExpr(pParse, TK_EQ, pE1, pE2)); + i = sqlite3VdbeAddOp4Int(v, OP_Found, iDataCur, 0, iPk, nPk); + VdbeCoverage(v); + sqlite3VdbeAddOp2(v, OP_Halt, SQLITE_CORRUPT, OE_Abort); + sqlite3VdbeJumpHere(v, i); } } /* pUpsert does not own pUpsertSrc - the outer INSERT statement does. So ** we have to make a copy before passing it down into sqlite3Update() */ pSrc = sqlite3SrcListDup(db, pUpsert->pUpsertSrc, 0); sqlite3Update(pParse, pSrc, pUpsert->pUpsertSet, - pWhere, OE_Abort, 0, 0, pUpsert); - pUpsert->pUpsertSet = 0; /* Will have been deleted by sqlite3Update() */ + pUpsert->pUpsertWhere, OE_Abort, 0, 0, pUpsert); + pUpsert->pUpsertSet = 0; /* Will have been deleted by sqlite3Update() */ + pUpsert->pUpsertWhere = 0; /* Will have been deleted by sqlite3Update() */ VdbeNoopComment((v, "End DO UPDATE of UPSERT")); } -- 2.47.2