[patches] [PATCH V3 09/10] sql: remove unnecessary VDBE flags
Bulat Niatshin
niatshin at tarantool.org
Mon Mar 5 14:53:28 MSK 2018
This commit is a part of work which is necessary for
successful removal of OP_Transaction. During that
opcode execution it doesn't do any meaningful work,
however it presence was necessary, because without
that opcode VDBE flags like 'readOnly', 'bIsReader'
would be assigned incorrectly.
This flags are unnecessary for us, so in this patch
they were removed (and all related to them).
For #3119
---
src/box/sql/sqlite3.h | 31 ---------------------------
src/box/sql/sqliteInt.h | 2 --
src/box/sql/vdbe.c | 56 ++++++++-----------------------------------------
src/box/sql/vdbeInt.h | 2 --
src/box/sql/vdbeapi.c | 16 +-------------
src/box/sql/vdbeaux.c | 47 +++++++++--------------------------------
6 files changed, 20 insertions(+), 134 deletions(-)
diff --git a/src/box/sql/sqlite3.h b/src/box/sql/sqlite3.h
index d771bddd2..f48a48624 100644
--- a/src/box/sql/sqlite3.h
+++ b/src/box/sql/sqlite3.h
@@ -3253,37 +3253,6 @@ sqlite3_sql(sqlite3_stmt * pStmt);
SQLITE_API char *
sqlite3_expanded_sql(sqlite3_stmt * pStmt);
-/*
- * CAPI3REF: Determine If An SQL Statement Writes The Database
- * METHOD: sqlite3_stmt
- *
- * ^The sqlite3_stmt_readonly(X) interface returns true (non-zero) if
- * and only if the [prepared statement] X makes no direct changes to
- * the content of the database file.
- *
- * Note that [application-defined SQL functions] might change the
- * database indirectly as a side effect.
- * ^(For example, if an application defines a function "eval()" that
- * calls [sqlite3_exec()], then the following SQL statement would
- * change the database file through side-effects:
- *
- * <blockquote><pre>
- * SELECT eval('DELETE FROM t1') FROM t2;
- * </pre></blockquote>
- *
- * But because the [SELECT] statement does not change the database file
- * directly, sqlite3_stmt_readonly() would still return true.)^
- *
- * ^Transaction control statements such as [BEGIN], [COMMIT], [ROLLBACK],
- * [SAVEPOINT], and [RELEASE] cause sqlite3_stmt_readonly() to return true,
- * since the statements themselves do not actually modify the database but
- * rather they control the timing of when other statements modify the
- * ^The sqlite3_stmt_readonly() interface returns true for [BEGIN] since
- * [BEGIN] merely sets internal flags.
-*/
-SQLITE_API int
-sqlite3_stmt_readonly(sqlite3_stmt * pStmt);
-
/*
* CAPI3REF: Determine If A Prepared Statement Has Been Reset
* METHOD: sqlite3_stmt
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 0f5303c58..fe67988b9 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -984,8 +984,6 @@ struct sqlite3 {
u8 imposterTable; /* Building an imposter table */
} init;
int nVdbeActive; /* Number of VDBEs currently running */
- int nVdbeRead; /* Number of active VDBEs that read or write */
- int nVdbeWrite; /* Number of active VDBEs that read and write */
int nVdbeExec; /* Number of nested calls to VdbeExec() */
int (*xTrace) (u32, void *, void *, void *); /* Trace function */
void *pTraceArg; /* Argument to the trace function */
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 7391952f3..7b4e0ae3b 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -620,7 +620,6 @@ int sqlite3VdbeExec(Vdbe *p)
goto no_mem;
}
assert(p->rc==SQLITE_OK || (p->rc&0xff)==SQLITE_BUSY);
- assert(p->bIsReader || p->readOnly!=0);
p->rc = SQLITE_OK;
p->iCurrentTime = 0;
assert(p->explain==0);
@@ -2824,22 +2823,13 @@ case OP_Savepoint: {
*/
assert(psql_txn->pSavepoint == 0 || p->autoCommit == 0);
assert(p1==SAVEPOINT_BEGIN||p1==SAVEPOINT_RELEASE||p1==SAVEPOINT_ROLLBACK);
- assert(p->bIsReader);
if (p1==SAVEPOINT_BEGIN) {
- if (db->nVdbeWrite>0) {
- /* A new savepoint cannot be created if there are active write
- * statements (i.e. open read/write incremental blob handles).
- */
- sqlite3VdbeError(p, "cannot open savepoint - SQL statements in progress");
- rc = SQLITE_BUSY;
- } else {
- /* Create a new savepoint structure. */
- pNew = sql_savepoint(p, zName);
- /* Link the new savepoint into the database handle's list. */
- pNew->pNext = psql_txn->pSavepoint;
- psql_txn->pSavepoint = pNew;
- }
+ /* Create a new savepoint structure. */
+ pNew = sql_savepoint(p, zName);
+ /* Link the new savepoint into the database handle's list. */
+ pNew->pNext = psql_txn->pSavepoint;
+ psql_txn->pSavepoint = pNew;
} else {
/* Find the named savepoint. If there is no such savepoint, then an
* an error is returned to the user.
@@ -2852,7 +2842,7 @@ case OP_Savepoint: {
if (!pSavepoint) {
sqlite3VdbeError(p, "no such savepoint: %s", zName);
rc = SQLITE_ERROR;
- } else if (db->nVdbeWrite>0 && p1==SAVEPOINT_RELEASE) {
+ } else if (p1==SAVEPOINT_RELEASE) {
/* It is not possible to release (commit) a savepoint if there are
* active write statements.
*/
@@ -2972,7 +2962,6 @@ case OP_AutoCommit: {
assert(desiredAutoCommit==1 || desiredAutoCommit==0);
assert(desiredAutoCommit==1 || iRollback==0);
assert(db->nVdbeActive>0); /* At least this one VM is active */
- assert(p->bIsReader);
if (desiredAutoCommit!=p->autoCommit) {
if (iRollback) {
@@ -2980,14 +2969,6 @@ case OP_AutoCommit: {
box_txn_rollback();
sqlite3RollbackAll(p, SQLITE_ABORT_ROLLBACK);
p->autoCommit = 1;
- } else if (desiredAutoCommit && db->nVdbeWrite>0) {
- /* If this instruction implements a COMMIT and other VMs are writing
- * return an error indicating that the other VMs must complete first.
- */
- sqlite3VdbeError(p, "cannot commit transaction - "
- "SQL statements in progress");
- rc = SQLITE_BUSY;
- goto abort_due_to_error;
} else if ((rc = sqlite3VdbeCheckFk(p, 1))!=SQLITE_OK) {
goto vdbe_return;
} else {
@@ -3041,8 +3022,6 @@ case OP_AutoCommit: {
*
*/
case OP_Transaction: {
- assert(p->bIsReader);
- assert(p->readOnly==0 || pOp->p2==0);
assert(pOp->p1==0);
if (pOp->p2 && (user_session->sql_flags & SQLITE_QueryOnly)!=0) {
rc = SQLITE_READONLY;
@@ -3096,8 +3075,6 @@ case OP_TTransaction: {
* executing this instruction.
*/
case OP_ReadCookie: { /* out2 */
- assert(p->bIsReader);
-
pOut = out2Prerelease(p, pOp);
pOut->u.i = 0;
break;
@@ -3114,7 +3091,6 @@ case OP_ReadCookie: { /* out2 */
*/
case OP_SetCookie: {
assert(pOp->p1==0);
- assert(p->readOnly==0);
/* See note about index shifting on OP_ReadCookie */
/* When the schema cookie changes, record the new cookie internally */
user_session->sql_flags |= SQLITE_InternChanges;
@@ -3211,9 +3187,6 @@ case OP_OpenRead:
case OP_OpenWrite:
assert(pOp->opcode==OP_OpenWrite || pOp->p5==0 || pOp->p5==OPFLAG_SEEKEQ);
- assert(p->bIsReader);
- assert(pOp->opcode==OP_OpenRead || pOp->opcode==OP_ReopenIdx
- || p->readOnly==0);
if (p->expired) {
rc = SQLITE_ABORT_ROLLBACK;
@@ -4668,22 +4641,12 @@ case OP_IdxGE: { /* jump */
* See also: Clear
*/
case OP_Destroy: { /* out2 */
- int iMoved;
-
- assert(p->readOnly==0);
assert(pOp->p1>1);
pOut = out2Prerelease(p, pOp);
pOut->flags = MEM_Null;
- if (db->nVdbeRead > 1) {
- rc = SQLITE_LOCKED;
- p->errorAction = ON_CONFLICT_ACTION_ABORT;
- goto abort_due_to_error;
- } else {
- iMoved = 0; /* Not needed. Only to silence a warning. */
- pOut->flags = MEM_Int;
- pOut->u.i = iMoved;
- if (rc) goto abort_due_to_error;
- }
+ pOut->flags = MEM_Int;
+ pOut->u.i = 0;
+ if (rc) goto abort_due_to_error;
break;
}
@@ -4706,7 +4669,6 @@ case OP_Destroy: { /* out2 */
* See also: Destroy
*/
case OP_Clear: {
- assert(p->readOnly==0);
rc = tarantoolSqlite3ClearTable(pOp->p1);
if (rc) goto abort_due_to_error;
break;
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index 1f7fdd37b..1836673a4 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -427,8 +427,6 @@ struct Vdbe {
bft changeCntOn:1; /* True to update the change-counter */
bft runOnlyOnce:1; /* Automatically expire on reset */
bft usesStmtJournal:1; /* True if uses a statement journal */
- bft readOnly:1; /* True for statements that do not write */
- bft bIsReader:1; /* True for statements that read */
bft isPrepareV2:1; /* True if prepared with prepare_v2() */
u32 aCounter[5]; /* Counters used by sqlite3_stmt_status() */
char *zSql; /* Text of the SQL statement that generated this */
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 550c19315..9c86c4c72 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -587,7 +587,7 @@ sqlite3Step(Vdbe * p)
db->u1.isInterrupted = 0;
}
- assert(db->nVdbeWrite > 0 || p->autoCommit == 0
+ assert(p->autoCommit == 0
|| (p->nDeferredCons == 0 && p->nDeferredImmCons == 0)
);
@@ -601,10 +601,6 @@ sqlite3Step(Vdbe * p)
#endif
db->nVdbeActive++;
- if (p->readOnly == 0)
- db->nVdbeWrite++;
- if (p->bIsReader)
- db->nVdbeRead++;
p->pc = 0;
}
#ifdef SQLITE_DEBUG
@@ -1558,16 +1554,6 @@ sqlite3_db_handle(sqlite3_stmt * pStmt)
return pStmt ? ((Vdbe *) pStmt)->db : 0;
}
-/*
- * Return true if the prepared statement is guaranteed to not modify the
- * database.
- */
-int
-sqlite3_stmt_readonly(sqlite3_stmt * pStmt)
-{
- return pStmt ? ((Vdbe *) pStmt)->readOnly : 1;
-}
-
/*
* Return true if the prepared statement is in need of being reset.
*/
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index 6622a11a8..91e7496de 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -639,12 +639,9 @@ sqlite3VdbeAssertMayAbort(Vdbe * v, int mayAbort)
* (2) Compute the maximum number of arguments used by any SQL function
* and store that value in *pMaxFuncArgs.
*
- * (3) Update the Vdbe.readOnly and Vdbe.bIsReader flags to accurately
- * indicate what the prepared statement actually does.
+ * (3) Initialize the p4.xAdvance pointer on opcodes that use it.
*
- * (4) Initialize the p4.xAdvance pointer on opcodes that use it.
- *
- * (5) Reclaim the memory allocated for storing labels.
+ * (4) Reclaim the memory allocated for storing labels.
*
* This routine will only function correctly if the mkopcodeh.tcl generator
* script numbers the opcodes correctly. Changes to this routine must be
@@ -657,8 +654,6 @@ resolveP2Values(Vdbe * p, int *pMaxFuncArgs)
Op *pOp;
Parse *pParse = p->pParse;
int *aLabel = pParse->aLabel;
- p->readOnly = 1;
- p->bIsReader = 0;
pOp = &p->aOp[p->nOp - 1];
while (1) {
@@ -673,17 +668,6 @@ resolveP2Values(Vdbe * p, int *pMaxFuncArgs)
* cases from this switch!
*/
switch (pOp->opcode) {
- case OP_Transaction:{
- if (pOp->p2 != 0)
- p->readOnly = 0;
- /* fall thru */
- FALLTHROUGH;
- }
- case OP_AutoCommit:
- case OP_Savepoint:{
- p->bIsReader = 1;
- break;
- }
case OP_Next:
case OP_NextIfOpen:
case OP_SorterNext:{
@@ -2420,22 +2404,14 @@ checkActiveVdbeCnt(sqlite3 * db)
{
Vdbe *p;
int cnt = 0;
- int nWrite = 0;
- int nRead = 0;
p = db->pVdbe;
while (p) {
if (sqlite3_stmt_busy((sqlite3_stmt *) p)) {
cnt++;
- if (p->readOnly == 0)
- nWrite++;
- if (p->bIsReader)
- nRead++;
}
p = p->pNext;
}
assert(cnt == db->nVdbeActive);
- assert(nWrite == db->nVdbeWrite);
- assert(nRead == db->nVdbeRead);
}
#else
#define checkActiveVdbeCnt(x)
@@ -2591,7 +2567,7 @@ sqlite3VdbeHalt(Vdbe * p)
/* No commit or rollback needed if the program never started or if the
* SQL statement does not read or write a database file.
*/
- if (p->pc >= 0 && p->bIsReader) {
+ if (p->pc >= 0) {
int mrc; /* Primary error code from p->rc */
int eStatementOp = 0;
int isSpecialError; /* Set to true if a 'special' error */
@@ -2613,7 +2589,7 @@ sqlite3VdbeHalt(Vdbe * p)
* pagerStress() in pager.c), the rollback is required to restore
* the pager to a consistent state.
*/
- if (!p->readOnly || mrc != SQLITE_INTERRUPT) {
+ if (mrc != SQLITE_INTERRUPT) {
if ((mrc == SQLITE_NOMEM || mrc == SQLITE_FULL)
&& p->autoCommit == 0) {
eStatementOp = SAVEPOINT_ROLLBACK;
@@ -2649,7 +2625,11 @@ sqlite3VdbeHalt(Vdbe * p)
&& !isSpecialError)) {
rc = sqlite3VdbeCheckFk(p, 1);
if (rc != SQLITE_OK) {
- if (NEVER(p->readOnly)) {
+ /* Close all opened cursors if
+ * they exist and free all
+ * VDBE frames.
+ */
+ if (NEVER(p->pDelFrame)) {
closeCursorsAndFree(p);
return SQLITE_ERROR;
}
@@ -2665,7 +2645,7 @@ sqlite3VdbeHalt(Vdbe * p)
SQL_TARANTOOL_ERROR;
closeCursorsAndFree(p);
}
- if (rc == SQLITE_BUSY && p->readOnly) {
+ if (rc == SQLITE_BUSY && !p->pDelFrame) {
closeCursorsAndFree(p);
return SQLITE_BUSY;
} else if (rc != SQLITE_OK) {
@@ -2745,13 +2725,6 @@ sqlite3VdbeHalt(Vdbe * p)
/* We have successfully halted and closed the VM. Record this fact. */
if (p->pc >= 0) {
db->nVdbeActive--;
- if (!p->readOnly)
- db->nVdbeWrite--;
- if (p->bIsReader)
- db->nVdbeRead--;
- assert(db->nVdbeActive >= db->nVdbeRead);
- assert(db->nVdbeRead >= db->nVdbeWrite);
- assert(db->nVdbeWrite >= 0);
}
p->magic = VDBE_MAGIC_HALT;
checkActiveVdbeCnt(db);
--
2.14.1
More information about the Tarantool-patches
mailing list