[tarantool-patches] [PATCH v1 24/28] sql: replace rc by is_aborted in struct VDBE

[imeevma@tarantool.org]

Currently, the rc field of the VDBE structure is either 0 or -1.
Due to this, it is better to replace this integer field with the
boolean field is_aborted.
---
 src/box/sql/vdbe.c    | 14 +++++++-------
 src/box/sql/vdbeInt.h |  3 ++-
 src/box/sql/vdbeapi.c | 10 +++++-----
 src/box/sql/vdbeaux.c | 34 +++++++++++++++-------------------
 4 files changed, 29 insertions(+), 32 deletions(-)

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 68edbc0..4204637 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -746,8 +746,7 @@ int sqlVdbeExec(Vdbe *p)
 	/*** INSERT STACK UNION HERE ***/
 
 	assert(p->magic==VDBE_MAGIC_RUN);  /* sql_step() verifies this */
-	assert(p->rc == 0);
-	p->rc = 0;
+	assert(!p->is_aborted);
 	p->iCurrentTime = 0;
 	assert(p->explain==0);
 	p->pResultSet = 0;
@@ -1052,16 +1051,17 @@ case OP_Halt: {
 		pOp = &aOp[pcx];
 		break;
 	}
-	p->rc = pOp->p1;
+	if (pOp->p1 != 0)
+		p->is_aborted = true;
 	p->errorAction = (u8)pOp->p2;
 	p->pc = pcx;
-	if (p->rc) {
+	if (p->is_aborted) {
 		if (pOp->p4.z != NULL)
 			box_error_set(__FILE__, __LINE__, pOp->p3, pOp->p4.z);
 		assert(! diag_is_empty(diag_get()));
 	}
 	sqlVdbeHalt(p);
-	rc = p->rc ? -1 : SQL_DONE;
+	rc = p->is_aborted ? -1 : SQL_DONE;
 	goto vdbe_return;
 }
 
@@ -2872,7 +2872,7 @@ case OP_Savepoint: {
 					goto vdbe_return;
 				}
 				sqlVdbeHalt(p);
-				if (p->rc != 0)
+				if (p->is_aborted)
 					goto abort_due_to_error;
 			} else {
 				if (p1==SAVEPOINT_ROLLBACK)
@@ -5259,7 +5259,7 @@ default: {          /* This is really OP_Noop and OP_Explain */
 	 */
 abort_due_to_error:
 	rc = -1;
-	p->rc = rc;
+	p->is_aborted = true;
 
 	/* This is the only way out of this procedure. */
 vdbe_return:
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index 617f16f..a1467c9 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -344,7 +344,8 @@ struct Vdbe {
 	int nCursor;		/* Number of slots in apCsr[] */
 	u32 cacheCtr;		/* VdbeCursor row cache generation counter */
 	int pc;			/* The program counter */
-	int rc;			/* Value to return */
+	/** True, if error occured during VDBE execution. */
+	bool is_aborted;
 	int nChange;		/* Number of db changes made since last reset */
 	int iStatement;		/* Statement number (or 0 if has not opened stmt) */
 	i64 iCurrentTime;	/* Value of julianday('now') for this statement */
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 9380155..03f5069 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -421,12 +421,12 @@ sqlStep(Vdbe * p)
 	/* Check that malloc() has not failed. If it has, return early. */
 	db = p->db;
 	if (db->mallocFailed) {
-		p->rc = -1;
+		p->is_aborted = true;
 		return -1;
 	}
 
 	if (p->pc <= 0 && p->expired) {
-		p->rc = -1;
+		p->is_aborted = true;
 		return -1;
 	}
 	if (p->pc < 0) {
@@ -459,10 +459,10 @@ sqlStep(Vdbe * p)
 
 	if (p->isPrepareV2 && rc != SQL_ROW && rc != SQL_DONE) {
 		/* If this statement was prepared using sql_prepare_v2(), and an
-		 * error has occurred, then return the error code in p->rc to the
-		 * caller. Set the error code in the database handle to the same value.
+		 * error has occurred, then return an error.
 		 */
-		rc = p->rc;
+		if (p->is_aborted)
+			rc = -1;
 	}
 	return rc;
 }
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index 28b423d..ec303c7 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -1460,7 +1460,6 @@ sqlVdbeList(Vdbe * p)
 		i = p->pc++;
 	} while (i < nRow && p->explain == 2 && p->aOp[i].opcode != OP_Explain);
 	if (i >= nRow) {
-		p->rc = 0;
 		rc = SQL_DONE;
 	} else {
 		char *zP4;
@@ -1569,7 +1568,6 @@ sqlVdbeList(Vdbe * p)
 
 		p->nResColumn = 8 - 4 * (p->explain - 1);
 		p->pResultSet = &p->aMem[1];
-		p->rc = 0;
 		rc = SQL_ROW;
 	}
 	return rc;
@@ -1703,7 +1701,7 @@ sqlVdbeRewind(Vdbe * p)
 	}
 #endif
 	p->pc = -1;
-	p->rc = 0;
+	p->is_aborted = false;
 	p->ignoreRaised = 0;
 	p->errorAction = ON_CONFLICT_ACTION_ABORT;
 	p->nChange = 0;
@@ -2088,7 +2086,7 @@ sqlVdbeCheckFk(Vdbe * p, int deferred)
 	if ((deferred && txn != NULL && txn->psql_txn != NULL &&
 	     txn->psql_txn->fk_deferred_count > 0) ||
 	    (!deferred && p->nFkConstraint > 0)) {
-		p->rc = -1;
+		p->is_aborted = true;
 		p->errorAction = ON_CONFLICT_ACTION_ABORT;
 		diag_set(ClientError, ER_SQL_EXECUTE, "FOREIGN KEY constraint "\
 			 "failed");
@@ -2164,7 +2162,7 @@ sqlVdbeHalt(Vdbe * p)
 	 */
 
 	if (db->mallocFailed) {
-		p->rc = -1;
+		p->is_aborted = true;
 	}
 	closeTopFrameCursors(p);
 	if (p->magic != VDBE_MAGIC_RUN) {
@@ -2179,7 +2177,7 @@ sqlVdbeHalt(Vdbe * p)
 		int eStatementOp = 0;
 
 		/* Check for immediate foreign key violations. */
-		if (p->rc == 0) {
+		if (!p->is_aborted) {
 			sqlVdbeCheckFk(p, 0);
 		}
 
@@ -2190,7 +2188,7 @@ sqlVdbeHalt(Vdbe * p)
 		 * above has occurred.
 		 */
 		if (p->auto_commit) {
-			if (p->rc == 0
+			if (!p->is_aborted
 			    || (p->errorAction == ON_CONFLICT_ACTION_FAIL)) {
 				rc = sqlVdbeCheckFk(p, 1);
 				if (rc != 0) {
@@ -2214,7 +2212,7 @@ sqlVdbeHalt(Vdbe * p)
 					closeCursorsAndFree(p);
 				}
 				if (rc != 0) {
-					p->rc = rc;
+					p->is_aborted = true;
 					box_txn_rollback();
 					closeCursorsAndFree(p);
 					sqlRollbackAll(p);
@@ -2228,7 +2226,8 @@ sqlVdbeHalt(Vdbe * p)
 			}
 			p->anonymous_savepoint = NULL;
 		} else if (eStatementOp == 0) {
-			if (p->rc == 0 || p->errorAction == ON_CONFLICT_ACTION_FAIL) {
+			if (!p->is_aborted ||
+			    p->errorAction == ON_CONFLICT_ACTION_FAIL) {
 				eStatementOp = SAVEPOINT_RELEASE;
 			} else if (p->errorAction == ON_CONFLICT_ACTION_ABORT) {
 				eStatementOp = SAVEPOINT_ROLLBACK;
@@ -2251,8 +2250,7 @@ sqlVdbeHalt(Vdbe * p)
 			rc = sqlVdbeCloseStatement(p, eStatementOp);
 			if (rc) {
 				box_txn_rollback();
-				if (p->rc == 0)
-					p->rc = rc;
+				p->is_aborted = true;
 				closeCursorsAndFree(p);
 				sqlRollbackAll(p);
 				sqlCloseSavepoints(p);
@@ -2282,9 +2280,8 @@ sqlVdbeHalt(Vdbe * p)
 	}
 	p->magic = VDBE_MAGIC_HALT;
 	checkActiveVdbeCnt(db);
-	if (db->mallocFailed) {
-		p->rc = -1;
-	}
+	if (db->mallocFailed)
+		p->is_aborted = true;
 
 	assert(db->nVdbeActive > 0 || box_txn() ||
 	       p->anonymous_savepoint == NULL);
@@ -2292,13 +2289,12 @@ sqlVdbeHalt(Vdbe * p)
 }
 
 /*
- * Each VDBE holds the result of the most recent sql_step() call
- * in p->rc.  This routine sets that result back to 0.
+ * This routine sets is_aborted of VDBE to false.
  */
 void
 sqlVdbeResetStepResult(Vdbe * p)
 {
-	p->rc = 0;
+	p->is_aborted = false;
 }
 
 /*
@@ -2337,7 +2333,7 @@ sqlVdbeReset(Vdbe * p)
 		 * is currently disabled, so this error has been
 		 * replaced with assert.
 		 */
-		assert(p->rc == 0 || p->expired == 0);
+		assert(!p->is_aborted || p->expired == 0);
 	}
 
 	/* Reclaim all memory used by the VDBE
@@ -2386,7 +2382,7 @@ sqlVdbeReset(Vdbe * p)
 #endif
 	p->iCurrentTime = 0;
 	p->magic = VDBE_MAGIC_RESET;
-	return p->rc;
+	return p->is_aborted ? -1 : 0;
 }
 
 /*
-- 
2.7.4