[tarantool-patches] [PATCH v1 20/21] sql: replace rc by is_aborted in struct Parse

[imeevma@tarantool.org]

Currently, the rc field of the Parse 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/vdbe.h    |  1 -
 src/box/sql/vdbeInt.h |  3 ++-
 src/box/sql/vdbeapi.c | 10 +++++-----
 src/box/sql/vdbeaux.c | 38 ++++++++++++--------------------------
 5 files changed, 26 insertions(+), 40 deletions(-)

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 237daef..af5b988 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -632,8 +632,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;
@@ -958,16 +957,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)
 			diag_set(ClientError, pOp->p5, 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;
 }
 
@@ -2881,7 +2881,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)
@@ -5274,7 +5274,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/vdbe.h b/src/box/sql/vdbe.h
index 6234dff..c8f2618 100644
--- a/src/box/sql/vdbe.h
+++ b/src/box/sql/vdbe.h
@@ -253,7 +253,6 @@ void sqlVdbeMakeReady(Vdbe *, Parse *);
 int sqlVdbeFinalize(Vdbe *);
 void sqlVdbeResolveLabel(Vdbe *, int);
 int sqlVdbeCurrentAddr(Vdbe *);
-void sqlVdbeResetStepResult(Vdbe *);
 void sqlVdbeRewind(Vdbe *);
 int sqlVdbeReset(Vdbe *);
 void sqlVdbeSetNumCols(Vdbe *, int);
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index 59a5cc8..3f8c9bd 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -348,7 +348,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 1842e26..0a4f45f 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -431,12 +431,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) {
@@ -469,10 +469,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..944a8a7 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,6 @@ sqlVdbeRewind(Vdbe * p)
 	}
 #endif
 	p->pc = -1;
-	p->rc = 0;
 	p->ignoreRaised = 0;
 	p->errorAction = ON_CONFLICT_ACTION_ABORT;
 	p->nChange = 0;
@@ -2088,7 +2085,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 +2161,7 @@ sqlVdbeHalt(Vdbe * p)
 	 */
 
 	if (db->mallocFailed) {
-		p->rc = -1;
+		p->is_aborted = true;
 	}
 	closeTopFrameCursors(p);
 	if (p->magic != VDBE_MAGIC_RUN) {
@@ -2179,7 +2176,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 +2187,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 +2211,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 +2225,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 +2249,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 +2279,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,16 +2288,6 @@ 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.
- */
-void
-sqlVdbeResetStepResult(Vdbe * p)
-{
-	p->rc = 0;
-}
-
-/*
  * Clean up a VDBE after execution but do not delete the VDBE just yet.
  * Return the result code.
  *
@@ -2337,7 +2323,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 +2372,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