[tarantool-patches] [PATCH 1/4] sql: remove OP_AutoCommit opcode
Nikita Pettik
korablev at tarantool.org
Thu May 3 21:49:22 MSK 2018
In SQLite OP_AutoCommit opcode used to set transaction operation:
BEGIN, ROLLBACK and COMMIT, switching auto-commit flag in VDBE.
As for Tarantool, it is confusing, since there are some differences
between auto-commit modes: 'INSERT ... VALUES (1), (2), (3)' is one
indivisible operation for SQLite, and three operations in real
auto-commit mode for Tarantool. To simulate SQLite auto-commit mode,
these three insertions are wrapped into one SEPARATE transaction,
which is, in fact, not real autocommit mode.
So, lets add separate explicit opcodes to BEGIN, ROLLBACK and COMMIT
transactions as user's operations. Auto-commit mode is set once at VDBE
creation and can be changed only by implicit opcode OP_TTransaction,
which is added to each DML statement, or by 'BEGIN' SQL statement.
---
src/box/sql/build.c | 51 +++++-------------
src/box/sql/main.c | 3 +-
src/box/sql/parse.y | 11 ++--
src/box/sql/sqliteInt.h | 31 +++++++++--
src/box/sql/vdbe.c | 136 +++++++++++++++++++++++++-----------------------
src/box/sql/vdbeInt.h | 3 +-
src/box/sql/vdbeapi.c | 5 +-
src/box/sql/vdbeaux.c | 34 ++++++------
8 files changed, 138 insertions(+), 136 deletions(-)
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index a2b712a4b..2794e09cd 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -3866,54 +3866,31 @@ sqlite3SrcListShiftJoinType(SrcList * p)
}
}
-/*
- * Generate VDBE code for a BEGIN statement.
- */
void
-sqlite3BeginTransaction(Parse * pParse, int MAYBE_UNUSED type)
+sql_transaction_begin(struct Parse *parse_context)
{
- sqlite3 MAYBE_UNUSED *db;
- Vdbe *v;
-
- assert(pParse != 0);
- db = pParse->db;
- assert(db != 0);
- v = sqlite3GetVdbe(pParse);
- if (!v)
- return;
- sqlite3VdbeAddOp0(v, OP_AutoCommit);
+ assert(parse_context != NULL);
+ struct Vdbe *v = sqlite3GetVdbe(parse_context);
+ if (v != NULL)
+ sqlite3VdbeAddOp0(v, OP_TransactionBegin);
}
-/*
- * Generate VDBE code for a COMMIT statement.
- */
void
-sqlite3CommitTransaction(Parse * pParse)
+sql_transaction_commit(struct Parse *parse_context)
{
- Vdbe *v;
-
- assert(pParse != 0);
- assert(pParse->db != 0);
- v = sqlite3GetVdbe(pParse);
- if (v) {
- sqlite3VdbeAddOp1(v, OP_AutoCommit, 1);
- }
+ assert(parse_context != NULL);
+ struct Vdbe *v = sqlite3GetVdbe(parse_context);
+ if (v != NULL)
+ sqlite3VdbeAddOp0(v, OP_TransactionCommit);
}
-/*
- * Generate VDBE code for a ROLLBACK statement.
- */
void
-sqlite3RollbackTransaction(Parse * pParse)
+sql_transaction_rollback(Parse *pParse)
{
- Vdbe *v;
-
assert(pParse != 0);
- assert(pParse->db != 0);
- v = sqlite3GetVdbe(pParse);
- if (v) {
- sqlite3VdbeAddOp2(v, OP_AutoCommit, 1, 1);
- }
+ struct Vdbe *v = sqlite3GetVdbe(pParse);
+ if (v != NULL)
+ sqlite3VdbeAddOp0(v, OP_TransactionRollback);
}
/*
diff --git a/src/box/sql/main.c b/src/box/sql/main.c
index 8e898178a..8775ef3ad 100644
--- a/src/box/sql/main.c
+++ b/src/box/sql/main.c
@@ -763,7 +763,6 @@ void
sqlite3RollbackAll(Vdbe * pVdbe, int tripCode)
{
sqlite3 *db = pVdbe->db;
- int inTrans = 0;
(void)tripCode;
struct session *user_session = current_session();
@@ -777,7 +776,7 @@ sqlite3RollbackAll(Vdbe * pVdbe, int tripCode)
user_session->sql_flags &= ~SQLITE_DeferFKs;
/* If one has been configured, invoke the rollback-hook callback */
- if (db->xRollbackCallback && (inTrans || !pVdbe->autoCommit)) {
+ if (db->xRollbackCallback && (!pVdbe->auto_commit)) {
db->xRollbackCallback(db->pRollbackArg);
}
}
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index b078e200d..bffd065fb 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -148,16 +148,13 @@ cmdx ::= cmd.
///////////////////// Begin and end transactions. ////////////////////////////
//
-cmd ::= BEGIN transtype(Y) trans_opt. {sqlite3BeginTransaction(pParse, Y);}
+cmd ::= BEGIN trans_opt. {sql_transaction_begin(pParse);}
trans_opt ::= .
trans_opt ::= TRANSACTION.
trans_opt ::= TRANSACTION nm.
-%type transtype {int}
-transtype(A) ::= . {A = TK_DEFERRED;}
-transtype(A) ::= DEFERRED(X). {A = @X; /*A-overwrites-X*/}
-cmd ::= COMMIT trans_opt. {sqlite3CommitTransaction(pParse);}
-cmd ::= END trans_opt. {sqlite3CommitTransaction(pParse);}
-cmd ::= ROLLBACK trans_opt. {sqlite3RollbackTransaction(pParse);}
+cmd ::= COMMIT trans_opt. {sql_transaction_commit(pParse);}
+cmd ::= END trans_opt. {sql_transaction_commit(pParse);}
+cmd ::= ROLLBACK trans_opt. {sql_transaction_rollback(pParse);}
savepoint_opt ::= SAVEPOINT.
savepoint_opt ::= .
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 8bb45c9d7..6bfe1352d 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -3662,9 +3662,34 @@ void sqlite3PrngSaveState(void);
void sqlite3PrngRestoreState(void);
#endif
void sqlite3RollbackAll(Vdbe *, int);
-void sqlite3BeginTransaction(Parse *, int);
-void sqlite3CommitTransaction(Parse *);
-void sqlite3RollbackTransaction(Parse *);
+
+/**
+ * Generate opcodes which start new Tarantool transaction.
+ * Used from parser to process BEGIN statement.
+ *
+ * @param parse_context Current parsing context.
+ */
+void
+sql_transaction_begin(struct Parse *parse_context);
+
+/**
+ * Generate opcodes which commit Tarantool transaction.
+ * Used from parser to process COMMIT statement.
+ *
+ * @param parse_context Current parsing context.
+ */
+void
+sql_transaction_commit(struct Parse *parse_context);
+
+/**
+ * Generate opcodes which rollback Tarantool transaction.
+ * Used from parser to process ROLLBACK statement.
+ *
+ * @param parse_context Current parsing context.
+ */
+void
+sql_transaction_rollback(struct Parse *parse_context);
+
void sqlite3Savepoint(Parse *, int, Token *);
void sqlite3CloseSavepoints(Vdbe *);
int sqlite3ExprIsConstant(Expr *);
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 013460f79..cf2b769c7 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2875,7 +2875,7 @@ case OP_Savepoint: {
/* Assert that the p1 parameter is valid. Also that if there is no open
* transaction, then there cannot be any savepoints.
*/
- assert(psql_txn->pSavepoint == 0 || p->autoCommit == 0);
+ assert(psql_txn->pSavepoint == 0 || box_txn());
assert(p1==SAVEPOINT_BEGIN||p1==SAVEPOINT_RELEASE||p1==SAVEPOINT_ROLLBACK);
if (p1==SAVEPOINT_BEGIN) {
@@ -2914,25 +2914,18 @@ case OP_Savepoint: {
if ((rc = sqlite3VdbeCheckFk(p, 1))!=SQLITE_OK) {
goto vdbe_return;
}
- p->autoCommit = 1;
if (sqlite3VdbeHalt(p)==SQLITE_BUSY) {
p->pc = (int)(pOp - aOp);
- p->autoCommit = 0;
p->rc = rc = SQLITE_BUSY;
goto vdbe_return;
}
rc = p->rc;
} else {
- int isSchemaChange;
if (p1==SAVEPOINT_ROLLBACK) {
- isSchemaChange = (user_session->sql_flags & SQLITE_InternChanges)!=0;
box_txn_rollback_to_savepoint(pSavepoint->tnt_savepoint);
- } else {
- isSchemaChange = 0;
- }
- if (isSchemaChange) {
- sqlite3ExpirePreparedStatements(db);
- user_session->sql_flags |= SQLITE_InternChanges;
+ if ((user_session->sql_flags &
+ SQLITE_InternChanges) != 0)
+ sqlite3ExpirePreparedStatements(db);
}
}
@@ -2997,53 +2990,60 @@ case OP_FkCheckCommit: {
break;
}
-/* Opcode: AutoCommit P1 P2 * * *
- *
- * Set the database auto-commit flag to P1 (1 or 0). If P2 is true, roll
- * back any currently active transactions. If there are any active
- * VMs (apart from this one), then a ROLLBACK fails. A COMMIT fails if
- * there are active writing VMs or active VMs that use shared cache.
- *
- * This instruction causes the VM to halt.
- */
-case OP_AutoCommit: {
- int desiredAutoCommit;
- int iRollback;
-
- desiredAutoCommit = pOp->p1;
- iRollback = pOp->p2;
- assert(desiredAutoCommit==1 || desiredAutoCommit==0);
- assert(desiredAutoCommit==1 || iRollback==0);
- assert(db->nVdbeActive>0); /* At least this one VM is active */
-
- if (desiredAutoCommit!=p->autoCommit) {
- if (iRollback) {
- assert(desiredAutoCommit==1);
- box_txn_rollback();
- sqlite3RollbackAll(p, SQLITE_ABORT_ROLLBACK);
- p->autoCommit = 1;
- } else if ((rc = sqlite3VdbeCheckFk(p, 1))!=SQLITE_OK) {
- goto vdbe_return;
- } else {
- p->autoCommit = (u8)desiredAutoCommit;
- if (p->autoCommit == 0) {
- sql_txn_begin(p);
- }
- }
- if (sqlite3VdbeHalt(p)==SQLITE_BUSY) {
- p->pc = (int)(pOp - aOp);
- p->autoCommit = (u8)(1-desiredAutoCommit);
- p->rc = rc = SQLITE_BUSY;
- goto vdbe_return;
+/* Opcode: TransactionBegin * * * * *
+ *
+ * Start Tarantool's transaction.
+ * Only do that if there is no other active transactions.
+ * Otherwise, raise an error with appropriate error message.
+ */
+case OP_TransactionBegin: {
+ if (!box_txn()) {
+ p->auto_commit = false;
+ rc = sql_txn_begin(p);
+ if (rc != SQLITE_OK)
+ goto abort_due_to_error;
+ } else {
+ sqlite3VdbeError(p, "cannot start a transaction within "
+ "a transaction");
+ rc = SQLITE_ERROR;
+ goto abort_due_to_error;
+ }
+ break;
+}
+
+/* Opcode: TransactionCommit * * * * *
+ *
+ * Commit Tarantool's transaction.
+ * If there is no active transaction, raise an error.
+ */
+case OP_TransactionCommit: {
+ if (box_txn()) {
+ if (box_txn_commit() != 0) {
+ rc = SQL_TARANTOOL_ERROR;
+ goto abort_due_to_error;
}
- rc = SQLITE_DONE;
- goto vdbe_return;
} else {
- sqlite3VdbeError(p,
- (!desiredAutoCommit)?"cannot start a transaction within a transaction":(
- (iRollback)?"cannot rollback - no transaction is active":
- "cannot commit - no transaction is active"));
+ sqlite3VdbeError(p, "cannot commit - no transaction is active");
+ rc = SQLITE_ERROR;
+ goto abort_due_to_error;
+ }
+ break;
+}
+/* Opcode: TransactionRollback * * * * *
+ *
+ * Rollback Tarantool's transaction.
+ * If there is no active transaction, raise an error.
+ */
+case OP_TransactionRollback: {
+ if (box_txn()) {
+ if (box_txn_rollback() != 0) {
+ rc = SQL_TARANTOOL_ERROR;
+ goto abort_due_to_error;
+ }
+ } else {
+ sqlite3VdbeError(p, "cannot rollback - no "
+ "transaction is active");
rc = SQLITE_ERROR;
goto abort_due_to_error;
}
@@ -3052,18 +3052,26 @@ case OP_AutoCommit: {
/* Opcode: TTransaction * * * * *
*
- * Start Tarantool's transaction.
- * Only do that if auto commit mode is on. This should be no-op
- * if this opcode was emitted inside a transaction.
+ * Start Tarantool's transaction, if there is no active
+ * transactions. Otherwise, create anonymous savepoint,
+ * which is used to correctly process ABORT statement inside
+ * outer transaction.
+ *
+ * In contrast to OP_TransactionBegin, this is service opcode,
+ * generated automatically alongside with DML routine.
*/
case OP_TTransaction: {
- if (p->autoCommit) {
- rc = box_txn_begin() == 0 ? SQLITE_OK : SQL_TARANTOOL_ERROR;
- }
- if (box_txn()
- && p->autoCommit == 0){
+ if (!box_txn()) {
+ if (box_txn_begin() != 0) {
+ rc = SQL_TARANTOOL_ERROR;
+ goto abort_due_to_error;
+ }
+ } else {
p->anonymous_savepoint = sql_savepoint(p, NULL);
-
+ if (p->anonymous_savepoint == NULL) {
+ rc = SQL_TARANTOOL_ERROR;
+ goto abort_due_to_error;
+ }
}
break;
}
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index ab9147c28..4b1767e24 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -381,7 +381,8 @@ struct Vdbe {
u8 ignoreRaised; /* Flag for ON CONFLICT IGNORE for triggers */
struct sql_txn *psql_txn; /* Data related to current transaction */
- u8 autoCommit; /* The auto-commit flag. */
+ /** The auto-commit flag. */
+ bool auto_commit;
/*
* `nDeferredCons` and `nDeferredImmCons` are stored in vdbe during
* vdbe execution and moved to sql_txn when it needs to be saved until
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 6e4185924..d8cdefa7c 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -560,9 +560,8 @@ sqlite3Step(Vdbe * p)
db->u1.isInterrupted = 0;
}
- assert(p->autoCommit == 0
- || (p->nDeferredCons == 0 && p->nDeferredImmCons == 0)
- );
+ assert(box_txn() ||
+ (p->nDeferredCons == 0 && p->nDeferredImmCons == 0));
#ifndef SQLITE_OMIT_TRACE
if ((db->xProfile || (db->mTrace & SQLITE_TRACE_PROFILE) != 0)
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index b3998ea27..0bd0d455b 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -65,9 +65,9 @@ sqlite3VdbeCreate(Parse * pParse)
db->pVdbe = p;
p->magic = VDBE_MAGIC_INIT;
p->pParse = pParse;
- p->autoCommit = (char)box_txn() == 0 ? 1 : 0;
+ p->auto_commit = !box_txn();
p->schema_ver = box_schema_version();
- if (!p->autoCommit) {
+ if (!p->auto_commit) {
p->psql_txn = in_txn()->psql_txn;
p->nDeferredCons = p->psql_txn->nDeferredConsSave;
p->nDeferredImmCons = p->psql_txn->nDeferredImmConsSave;
@@ -2477,31 +2477,27 @@ sqlite3VdbeCheckFk(Vdbe * p, int deferred)
int
sql_txn_begin(Vdbe *p)
{
- struct txn *ptxn;
-
if (in_txn()) {
diag_set(ClientError, ER_ACTIVE_TRANSACTION);
- return -1;
+ return SQL_TARANTOOL_ERROR;
}
- ptxn = txn_begin(false);
+ struct txn *ptxn = txn_begin(false);
if (ptxn == NULL)
- return -1;
+ return SQLITE_NOMEM;
ptxn->psql_txn = region_alloc_object(&fiber()->gc, struct sql_txn);
if (ptxn->psql_txn == NULL) {
box_txn_rollback();
- return -1;
+ return SQLITE_NOMEM;
}
memset(ptxn->psql_txn, 0, sizeof(struct sql_txn));
p->psql_txn = ptxn->psql_txn;
- return 0;
+ return SQLITE_OK;
}
Savepoint *
-sql_savepoint(Vdbe *p,
- const char *zName)
+sql_savepoint(Vdbe *p, const char *zName)
{
assert(p);
- assert(p->psql_txn);
size_t nName = zName ? strlen(zName) + 1 : 0;
size_t savepoint_sz = sizeof(Savepoint) + nName;
Savepoint *pNew;
@@ -2509,8 +2505,11 @@ sql_savepoint(Vdbe *p,
pNew = (Savepoint *)region_aligned_alloc(&fiber()->gc,
savepoint_sz,
alignof(Savepoint));
- if (!pNew)
+ if (pNew == NULL) {
+ diag_set(OutOfMemory, savepoint_sz, "region",
+ "savepoint");
return NULL;
+ }
pNew->tnt_savepoint = box_txn_savepoint();
if (!pNew->tnt_savepoint)
return NULL;
@@ -2595,7 +2594,7 @@ sqlite3VdbeHalt(Vdbe * p)
*/
if (mrc != SQLITE_INTERRUPT) {
if ((mrc == SQLITE_NOMEM || mrc == SQLITE_FULL)
- && p->autoCommit == 0) {
+ && box_txn()) {
eStatementOp = SAVEPOINT_ROLLBACK;
} else {
/* We are forced to roll back the active transaction. Before doing
@@ -2606,7 +2605,6 @@ sqlite3VdbeHalt(Vdbe * p)
sqlite3RollbackAll(p,
SQLITE_ABORT_ROLLBACK);
sqlite3CloseSavepoints(p);
- p->autoCommit = 1;
p->nChange = 0;
}
}
@@ -2623,7 +2621,7 @@ sqlite3VdbeHalt(Vdbe * p)
* Note: This block also runs if one of the special errors handled
* above has occurred.
*/
- if (p->autoCommit) {
+ if (p->auto_commit) {
if (p->rc == SQLITE_OK
|| (p->errorAction == ON_CONFLICT_ACTION_FAIL
&& !isSpecialError)) {
@@ -2682,7 +2680,6 @@ sqlite3VdbeHalt(Vdbe * p)
closeCursorsAndFree(p);
sqlite3RollbackAll(p, SQLITE_ABORT_ROLLBACK);
sqlite3CloseSavepoints(p);
- p->autoCommit = 1;
p->nChange = 0;
}
}
@@ -2706,7 +2703,6 @@ sqlite3VdbeHalt(Vdbe * p)
closeCursorsAndFree(p);
sqlite3RollbackAll(p, SQLITE_ABORT_ROLLBACK);
sqlite3CloseSavepoints(p);
- p->autoCommit = 1;
p->nChange = 0;
}
}
@@ -2742,7 +2738,7 @@ sqlite3VdbeHalt(Vdbe * p)
if (!box_txn())
fiber_gc();
- assert(db->nVdbeActive > 0 || p->autoCommit == 0 ||
+ assert(db->nVdbeActive > 0 || box_txn() ||
p->anonymous_savepoint == NULL);
return (p->rc == SQLITE_BUSY ? SQLITE_BUSY : SQLITE_OK);
}
--
2.15.1
More information about the Tarantool-patches
mailing list