[tarantool-patches] [PATCH v1 05/12] sql: remove error SQL_INTERRUPT

[imeevma@tarantool.org]

Since the interrupt system is no longer used in SQL, the
SQL_INTERRUPT error is out of date and should be removed. Also
this patch removes currently unused progress callback system.
---
 src/box/sql/main.c     |  1 -
 src/box/sql/malloc.c   |  4 ---
 src/box/sql/sqlInt.h   | 11 -------
 src/box/sql/tokenize.c |  3 --
 src/box/sql/vdbe.c     | 84 ++------------------------------------------------
 src/box/sql/vdbeapi.c  |  7 -----
 src/box/sql/vdbeaux.c  | 41 ++++++++++--------------
 7 files changed, 19 insertions(+), 132 deletions(-)

diff --git a/src/box/sql/main.c b/src/box/sql/main.c
index ed07553..0c6ad47 100644
--- a/src/box/sql/main.c
+++ b/src/box/sql/main.c
@@ -371,7 +371,6 @@ sqlErrStr(int rc)
 		/* SQL_BUSY        */ "database is locked",
 		/* SQL_LOCKED      */ "database table is locked",
 		/* SQL_NOMEM       */ "out of memory",
-		/* SQL_INTERRUPT   */ "interrupted",
 		/* SQL_IOERR       */ "disk I/O error",
 		/* SQL_NOTFOUND    */ "unknown operation",
 		/* SQL_FULL        */ "database or disk is full",
diff --git a/src/box/sql/malloc.c b/src/box/sql/malloc.c
index d6f99b4..dbc6846 100644
--- a/src/box/sql/malloc.c
+++ b/src/box/sql/malloc.c
@@ -898,9 +898,6 @@ sqlOomFault(sql * db)
 {
 	if (db->mallocFailed == 0 && db->bBenignMalloc == 0) {
 		db->mallocFailed = 1;
-		if (db->nVdbeExec > 0) {
-			db->u1.isInterrupted = 1;
-		}
 		db->lookaside.bDisable++;
 	}
 }
@@ -917,7 +914,6 @@ sqlOomClear(sql * db)
 {
 	if (db->mallocFailed && db->nVdbeExec == 0) {
 		db->mallocFailed = 0;
-		db->u1.isInterrupted = 0;
 		assert(db->lookaside.bDisable > 0);
 		db->lookaside.bDisable--;
 	}
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 5c71c51..7f47da6 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -370,8 +370,6 @@ enum sql_ret_code {
 	SQL_LOCKED,
 	/** A malloc() failed. */
 	SQL_NOMEM,
-	/** Operation terminated by sql_interrupt(). */
-	SQL_INTERRUPT,
 	/** Some kind of disk I/O error occurred. */
 	SQL_IOERR,
 	/** Unknown opcode in sql_file_control(). */
@@ -1468,16 +1466,7 @@ struct sql {
 	void (*xUpdateCallback) (void *, int, const char *, const char *,
 				 sql_int64);
 	sql_value *pErr;	/* Most recent error message */
-	union {
-		volatile int isInterrupted;	/* True if sql_interrupt has been called */
-		double notUsed1;	/* Spacer */
-	} u1;
 	Lookaside lookaside;	/* Lookaside malloc configuration */
-#ifndef SQL_OMIT_PROGRESS_CALLBACK
-	int (*xProgress) (void *);	/* The progress callback */
-	void *pProgressArg;	/* Argument to the progress callback */
-	unsigned nProgressOps;	/* Number of opcodes for progress callback */
-#endif
 	Hash aFunc;		/* Hash table of connection functions */
 	int *pnBytesFreed;	/* If not NULL, increment this in DbFree() */
 };
diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c
index 8cc3532..5cb4b7a 100644
--- a/src/box/sql/tokenize.c
+++ b/src/box/sql/tokenize.c
@@ -457,9 +457,6 @@ sqlRunParser(Parse * pParse, const char *zSql)
 
 	assert(zSql != 0);
 	mxSqlLen = db->aLimit[SQL_LIMIT_SQL_LENGTH];
-	if (db->nVdbeActive == 0) {
-		db->u1.isInterrupted = 0;
-	}
 	pParse->zTail = zSql;
 	i = 0;
 	/* sqlParserTrace(stdout, "parser: "); */
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 85cec85..c2eec93 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -92,18 +92,6 @@ int sql_xfer_count = 0;
 #endif
 
 /*
- * When this global variable is positive, it gets decremented once before
- * each instruction in the VDBE.  When it reaches zero, the u1.isInterrupted
- * field of the sql structure is set in order to simulate an interrupt.
- *
- * This facility is used for testing purposes only.  It does not function
- * in an ordinary build.
- */
-#ifdef SQL_TEST
-int sql_interrupt_count = 0;
-#endif
-
-/*
  * The next global variable is incremented each type the OP_Sort opcode
  * is executed.  The test procedures use this information to make sure that
  * sorting is occurring or not occurring at appropriate times.   This variable
@@ -631,9 +619,6 @@ int sqlVdbeExec(Vdbe *p)
 	sql *db = p->db;       /* The database */
 	int iCompare = 0;          /* Result of last comparison */
 	unsigned nVmStep = 0;      /* Number of virtual machine steps */
-#ifndef SQL_OMIT_PROGRESS_CALLBACK
-	unsigned nProgressLimit = 0;/* Invoke xProgress() when nVmStep reaches this */
-#endif
 	Mem *aMem = p->aMem;       /* Copy of p->aMem */
 	Mem *pIn1 = 0;             /* 1st input operand */
 	Mem *pIn2 = 0;             /* 2nd input operand */
@@ -658,15 +643,7 @@ int sqlVdbeExec(Vdbe *p)
 	p->iCurrentTime = 0;
 	assert(p->explain==0);
 	p->pResultSet = 0;
-	if (db->u1.isInterrupted) goto abort_due_to_interrupt;
 	sqlVdbeIOTraceSql(p);
-#ifndef SQL_OMIT_PROGRESS_CALLBACK
-	if (db->xProgress) {
-		u32 iPrior = p->aCounter[SQL_STMTSTATUS_VM_STEP];
-		assert(0 < db->nProgressOps);
-		nProgressLimit = db->nProgressOps - (iPrior % db->nProgressOps);
-	}
-#endif
 #ifdef SQL_DEBUG
 	sqlBeginBenignMalloc();
 	if (p->pc==0
@@ -810,40 +787,7 @@ int sqlVdbeExec(Vdbe *p)
  * to the current line should be indented for EXPLAIN output.
  */
 case OP_Goto: {             /* jump */
-			jump_to_p2_and_check_for_interrupt:
-	pOp = &aOp[pOp->p2 - 1];
-
-	/* Opcodes that are used as the bottom of a loop (OP_Next, OP_Prev,
-	 * OP_RowSetNext, or OP_SorterNext) all jump here upon
-	 * completion.  Check to see if sql_interrupt() has been called
-	 * or if the progress callback needs to be invoked.
-	 *
-	 * This code uses unstructured "goto" statements and does not look clean.
-	 * But that is not due to sloppy coding habits. The code is written this
-	 * way for performance, to avoid having to run the interrupt and progress
-	 * checks on every opcode.  This helps sql_step() to run about 1.5%
-	 * faster according to "valgrind --tool=cachegrind"
-	 */
-			check_for_interrupt:
-	if (db->u1.isInterrupted) goto abort_due_to_interrupt;
-#ifndef SQL_OMIT_PROGRESS_CALLBACK
-	/* Call the progress callback if it is configured and the required number
-	 * of VDBE ops have been executed (either since this invocation of
-	 * sqlVdbeExec() or since last time the progress callback was called).
-	 * If the progress callback returns non-zero, exit the virtual machine with
-	 * a return code SQL_ABORT.
-	 */
-	if (db->xProgress!=0 && nVmStep>=nProgressLimit) {
-		assert(db->nProgressOps!=0);
-		nProgressLimit = nVmStep + db->nProgressOps - (nVmStep%db->nProgressOps);
-		if (db->xProgress(db->pProgressArg)) {
-			rc = SQL_INTERRUPT;
-			goto abort_due_to_error;
-		}
-	}
-#endif
-
-	break;
+	goto jump_to_p2;
 }
 
 /* Opcode:  Gosub P1 P2 * * *
@@ -1387,18 +1331,6 @@ case OP_ResultRow: {
 	assert(pOp->p1>0);
 	assert(pOp->p1+pOp->p2<=(p->nMem+1 - p->nCursor)+1);
 
-#ifndef SQL_OMIT_PROGRESS_CALLBACK
-	/* Run the progress counter just before returning.
-	 */
-	if (db->xProgress!=0
-	    && nVmStep>=nProgressLimit
-	    && db->xProgress(db->pProgressArg)!=0
-		) {
-		rc = SQL_INTERRUPT;
-		goto abort_due_to_error;
-	}
-#endif
-
 	/* If this statement has violated immediate foreign key constraints, do
 	 * not return the number of rows modified. And do not RELEASE the statement
 	 * transaction. It needs to be rolled back.
@@ -4340,11 +4272,11 @@ case OP_Next:          /* jump */
 #ifdef SQL_TEST
 		sql_search_count++;
 #endif
-		goto jump_to_p2_and_check_for_interrupt;
+		goto jump_to_p2;
 	} else {
 		pC->nullRow = 1;
 	}
-	goto check_for_interrupt;
+	break;
 }
 
 /* Opcode: SorterInsert P1 P2 * * *
@@ -5491,14 +5423,4 @@ no_mem:
 	sqlVdbeError(p, "out of memory");
 	rc = SQL_NOMEM;
 	goto abort_due_to_error;
-
-	/* Jump to here if the sql_interrupt() API sets the interrupt
-	 * flag.
-	 */
-abort_due_to_interrupt:
-	assert(db->u1.isInterrupted);
-	rc = db->mallocFailed ? SQL_NOMEM : SQL_INTERRUPT;
-	p->rc = rc;
-	sqlVdbeError(p, "%s", sqlErrStr(rc));
-	goto abort_due_to_error;
 }
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index d286856..909db8c 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -537,13 +537,6 @@ sqlStep(Vdbe * p)
 		goto end_of_step;
 	}
 	if (p->pc < 0) {
-		/* If there are no other statements currently running, then
-		 * reset the interrupt flag.  This prevents a call to sql_interrupt
-		 * from interrupting a statement that has not yet started.
-		 */
-		if (db->nVdbeActive == 0) {
-			db->u1.isInterrupted = 0;
-		}
 
 #ifndef SQL_OMIT_TRACE
 		if ((db->xProfile || (db->mTrace & SQL_TRACE_PROFILE) != 0)
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index 140bb97..27fa5b2 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -1486,10 +1486,6 @@ sqlVdbeList(Vdbe * p)
 	if (i >= nRow) {
 		p->rc = SQL_OK;
 		rc = SQL_DONE;
-	} else if (db->u1.isInterrupted) {
-		p->rc = SQL_INTERRUPT;
-		rc = SQL_ERROR;
-		sqlVdbeError(p, sqlErrStr(p->rc));
 	} else {
 		char *zP4;
 		Op *pOp;
@@ -2208,7 +2204,6 @@ sqlVdbeHalt(Vdbe * p)
 	 *     SQL_NOMEM
 	 *     SQL_IOERR
 	 *     SQL_FULL
-	 *     SQL_INTERRUPT
 	 *
 	 * Then the internal cache might have been left in an inconsistent
 	 * state.  We need to rollback the statement transaction, if there is
@@ -2234,13 +2229,11 @@ sqlVdbeHalt(Vdbe * p)
 
 		/* Check for one of the special errors */
 		mrc = p->rc & 0xff;
-		isSpecialError = mrc == SQL_NOMEM || mrc == SQL_IOERR
-		    || mrc == SQL_INTERRUPT || mrc == SQL_FULL;
+		isSpecialError = mrc == SQL_NOMEM || mrc == SQL_IOERR ||
+				 mrc == SQL_FULL;
 		if (isSpecialError) {
-			/* If the query was read-only and the error code is SQL_INTERRUPT,
-			 * no rollback is necessary. Otherwise, at least a savepoint
-			 * transaction must be rolled back to restore the database to a
-			 * consistent state.
+			/* At least a savepoint transaction must be rolled back
+			 * to restore the database to a consistent state.
 			 *
 			 * Even if the statement is read-only, it is important to perform
 			 * a statement or transaction rollback operation. If the error
@@ -2249,20 +2242,18 @@ sqlVdbeHalt(Vdbe * p)
 			 * pagerStress() in pager.c), the rollback is required to restore
 			 * the pager to a consistent state.
 			 */
-			if (mrc != SQL_INTERRUPT) {
-				if ((mrc == SQL_NOMEM || mrc == SQL_FULL)
-				    && box_txn()) {
-					eStatementOp = SAVEPOINT_ROLLBACK;
-				} else {
-					/* We are forced to roll back the active transaction. Before doing
-					 * so, abort any other statements this handle currently has active.
-					 */
-					box_txn_rollback();
-					closeCursorsAndFree(p);
-					sqlRollbackAll(p);
-					sqlCloseSavepoints(p);
-					p->nChange = 0;
-				}
+			if ((mrc == SQL_NOMEM || mrc == SQL_FULL)
+			    && box_txn()) {
+				eStatementOp = SAVEPOINT_ROLLBACK;
+			} else {
+				/* We are forced to roll back the active transaction. Before doing
+				 * so, abort any other statements this handle currently has active.
+				 */
+				box_txn_rollback();
+				closeCursorsAndFree(p);
+				sqlRollbackAll(p);
+				sqlCloseSavepoints(p);
+				p->nChange = 0;
 			}
 		}
 
-- 
2.7.4