From 05b5f0994d4ca4538523837d17fad5f8c6ebff92 Mon Sep 17 00:00:00 2001
From: Mergen Imeev <imeevma@gmail.com>
Date: Fri, 12 Apr 2019 13:42:51 +0300
Subject: [PATCH] sql: remove mayAbort field from struct Parse
Currently, the mayAbort field is working with SQL error
SQL_CONSTRAINT. Since we want to replace SQL_CONSTRAINT with a
Tarantool error, we need to change the way the mayAbort field
works or delete it. Since this field is used only for make an
assertion in debug mode, it is better to simply remove it.
---
src/box/sql/build.c | 28 --------
src/box/sql/expr.c | 2 -
src/box/sql/fk_constraint.c | 35 ----------
src/box/sql/insert.c | 3 -
src/box/sql/sqlInt.h | 2 -
src/box/sql/vdbe.h | 3 -
src/box/sql/vdbeaux.c | 127 ------------------------------------
7 files changed, 200 deletions(-)
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 4a6a9a327b..c40685e445 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -154,8 +154,6 @@ sql_finish_coding(struct Parse *parse_context)
* Begin by generating some termination code at the end
* of the vdbe program
*/
- assert(!parse_context->isMultiWrite ||
- sqlVdbeAssertMayAbort(v, parse_context->mayAbort));
int last_instruction = v->nOp;
if (parse_context->initiateTTrans)
sqlVdbeAddOp0(v, OP_TTransaction);
@@ -3180,29 +3178,6 @@ sql_set_multi_write(struct Parse *parse_context, bool is_set)
pToplevel->isMultiWrite |= is_set;
}
-/*
- * The code generator calls this routine if is discovers that it is
- * possible to abort a statement prior to completion. In order to
- * perform this abort without corrupting the database, we need to make
- * sure that the statement is protected by a statement transaction.
- *
- * Technically, we only need to set the mayAbort flag if the
- * isMultiWrite flag was previously set. There is a time dependency
- * such that the abort must occur after the multiwrite. This makes
- * some statements involving the REPLACE conflict resolution algorithm
- * go a little faster. But taking advantage of this time dependency
- * makes it more difficult to prove that the code is correct (in
- * particular, it prevents us from writing an effective
- * implementation of sqlAssertMayAbort()) and so we have chosen
- * to take the safe route and skip the optimization.
- */
-void
-sqlMayAbort(Parse * pParse)
-{
- Parse *pToplevel = sqlParseToplevel(pParse);
- pToplevel->mayAbort = 1;
-}
-
/*
* Code an OP_Halt that causes the vdbe to return an SQL_CONSTRAINT
* error. The onError parameter determines which (if any) of the statement
@@ -3219,9 +3194,6 @@ sqlHaltConstraint(Parse * pParse, /* Parsing context */
{
Vdbe *v = sqlGetVdbe(pParse);
assert((errCode & 0xff) == SQL_CONSTRAINT);
- if (onError == ON_CONFLICT_ACTION_ABORT) {
- sqlMayAbort(pParse);
- }
sqlVdbeAddOp4(v, OP_Halt, errCode, onError, 0, p4, p4type);
sqlVdbeChangeP5(v, p5Errmsg);
}
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index a8a984404c..4bbfee63be 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -4338,8 +4338,6 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
pParse->is_aborted = true;
return 0;
}
- if (pExpr->on_conflict_action == ON_CONFLICT_ACTION_ABORT)
- sqlMayAbort(pParse);
assert(!ExprHasProperty(pExpr, EP_IntValue));
if (pExpr->on_conflict_action == ON_CONFLICT_ACTION_IGNORE) {
sqlVdbeAddOp4(v, OP_Halt, SQL_OK,
diff --git a/src/box/sql/fk_constraint.c b/src/box/sql/fk_constraint.c
index 3bcc0fd24f..04c694ae02 100644
--- a/src/box/sql/fk_constraint.c
+++ b/src/box/sql/fk_constraint.c
@@ -290,8 +290,6 @@ fk_constraint_lookup_parent(struct Parse *parse_context, struct space *parent,
ON_CONFLICT_ACTION_ABORT, 0, P4_STATIC,
P5_ConstraintFK);
} else {
- if (incr_count > 0 && !fk_def->is_deferred)
- sqlMayAbort(parse_context);
sqlVdbeAddOp2(v, OP_FkCounter, fk_def->is_deferred,
incr_count);
}
@@ -631,41 +629,8 @@ fk_constraint_emit_check(struct Parse *parser, struct space *space, int reg_old,
fk->def, reg_new, -1);
}
if (reg_old != 0) {
- enum fk_constraint_action action = fk_def->on_update;
fk_constraint_scan_children(parser, src, space->def,
fk->def, reg_old, 1);
- /*
- * If this is a deferred FK constraint, or
- * a CASCADE or SET NULL action applies,
- * then any foreign key violations caused
- * by removing the parent key will be
- * rectified by the action trigger. So do
- * not set the "may-abort" flag in this
- * case.
- *
- * Note 1: If the FK is declared "ON
- * UPDATE CASCADE", then the may-abort
- * flag will eventually be set on this
- * statement anyway (when this function is
- * called as part of processing the UPDATE
- * within the action trigger).
- *
- * Note 2: At first glance it may seem
- * like sql could simply omit all
- * OP_FkCounter related scans when either
- * CASCADE or SET NULL applies. The
- * trouble starts if the CASCADE or SET
- * NULL action trigger causes other
- * triggers or action rules attached to
- * the child table to fire. In these cases
- * the fk constraint counters might be set
- * incorrectly if any OP_FkCounter related
- * scans are omitted.
- */
- if (!fk_def->is_deferred &&
- action != FKEY_ACTION_CASCADE &&
- action != FKEY_ACTION_SET_NULL)
- sqlMayAbort(parser);
}
sqlSrcListDelete(db, src);
}
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 6d70c51c2d..1fc3032e48 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -808,7 +808,6 @@ vdbe_emit_ck_constraint(struct Parse *parser, struct Expr *expr,
ck_constraint_name));
int check_is_passed = sqlVdbeMakeLabel(v);
sqlExprIfTrue(parser, expr, check_is_passed, SQL_JUMPIFNULL);
- sqlMayAbort(parser);
const char *fmt = tnt_errcode_desc(ER_CK_CONSTRAINT_FAILED);
const char *error_msg = tt_sprintf(fmt, ck_constraint_name, expr_str);
sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, ON_CONFLICT_ACTION_ABORT,
@@ -858,8 +857,6 @@ vdbe_emit_constraint_checks(struct Parse *parse_context, struct space *space,
int addr;
switch (on_conflict_nullable) {
case ON_CONFLICT_ACTION_ABORT:
- sqlMayAbort(parse_context);
- FALLTHROUGH;
case ON_CONFLICT_ACTION_ROLLBACK:
case ON_CONFLICT_ACTION_FAIL:
err_msg = sqlMPrintf(db, "%s.%s", def->name,
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index eb2d4cbb8d..a8318231d6 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -2423,7 +2423,6 @@ struct Parse {
u8 colNamesSet; /* TRUE after OP_ColumnName has been issued to pVdbe */
u8 nTempReg; /* Number of temporary registers in aTempReg[] */
u8 isMultiWrite; /* True if statement may modify/insert multiple rows */
- u8 mayAbort; /* True if statement may throw an ABORT exception */
u8 hasCompound; /* Need to invoke convertCompoundSelectToSubquery() */
u8 okConstFactor; /* OK to factor out constants */
u8 disableLookaside; /* Number of times lookaside has been disabled */
@@ -3741,7 +3740,6 @@ vdbe_emit_insertion_completion(struct Vdbe *v, struct space *space,
void
sql_set_multi_write(Parse *, bool);
-void sqlMayAbort(Parse *);
void sqlHaltConstraint(Parse *, int, int, char *, i8, u8);
Expr *sqlExprDup(sql *, Expr *, int);
diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h
index a609e8cc84..4f94643cb7 100644
--- a/src/box/sql/vdbe.h
+++ b/src/box/sql/vdbe.h
@@ -248,9 +248,6 @@ void sqlVdbeMakeReady(Vdbe *, Parse *);
int sqlVdbeFinalize(Vdbe *);
void sqlVdbeResolveLabel(Vdbe *, int);
int sqlVdbeCurrentAddr(Vdbe *);
-#ifdef SQL_DEBUG
-int sqlVdbeAssertMayAbort(Vdbe *, int);
-#endif
void sqlVdbeResetStepResult(Vdbe *);
void sqlVdbeRewind(Vdbe *);
int sqlVdbeReset(Vdbe *);
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index a90c492fec..d8018ba52a 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -496,133 +496,6 @@ sqlVdbeRunOnlyOnce(Vdbe * p)
p->runOnlyOnce = 1;
}
-#ifdef SQL_DEBUG /* sqlAssertMayAbort() logic */
-
-/*
- * The following type and function are used to iterate through all opcodes
- * in a Vdbe main program and each of the sub-programs (triggers) it may
- * invoke directly or indirectly. It should be used as follows:
- *
- * Op *pOp;
- * VdbeOpIter sIter;
- *
- * memset(&sIter, 0, sizeof(sIter));
- * sIter.v = v; // v is of type Vdbe*
- * while( (pOp = opIterNext(&sIter)) ){
- * // Do something with pOp
- * }
- * sqlDbFree(v->db, sIter.apSub);
- *
- */
-typedef struct VdbeOpIter VdbeOpIter;
-struct VdbeOpIter {
- Vdbe *v; /* Vdbe to iterate through the opcodes of */
- SubProgram **apSub; /* Array of subprograms */
- int nSub; /* Number of entries in apSub */
- int iAddr; /* Address of next instruction to return */
- int iSub; /* 0 = main program, 1 = first sub-program etc. */
-};
-
-static Op *
-opIterNext(VdbeOpIter * p)
-{
- Vdbe *v = p->v;
- Op *pRet = 0;
- Op *aOp;
- int nOp;
-
- if (p->iSub <= p->nSub) {
-
- if (p->iSub == 0) {
- aOp = v->aOp;
- nOp = v->nOp;
- } else {
- aOp = p->apSub[p->iSub - 1]->aOp;
- nOp = p->apSub[p->iSub - 1]->nOp;
- }
- assert(p->iAddr < nOp);
-
- pRet = &aOp[p->iAddr];
- p->iAddr++;
- if (p->iAddr == nOp) {
- p->iSub++;
- p->iAddr = 0;
- }
-
- if (pRet->p4type == P4_SUBPROGRAM) {
- int nByte = (p->nSub + 1) * sizeof(SubProgram *);
- int j;
- for (j = 0; j < p->nSub; j++) {
- if (p->apSub[j] == pRet->p4.pProgram)
- break;
- }
- if (j == p->nSub) {
- p->apSub =
- sqlDbReallocOrFree(v->db, p->apSub,
- nByte);
- if (!p->apSub) {
- pRet = 0;
- } else {
- p->apSub[p->nSub++] = pRet->p4.pProgram;
- }
- }
- }
- }
-
- return pRet;
-}
-
-/*
- * Check if the program stored in the VM associated with pParse may
- * throw an ABORT exception (causing the statement, but not entire transaction
- * to be rolled back). This condition is true if the main program or any
- * sub-programs contains any of the following:
- *
- * * OP_Halt with P1=SQL_CONSTRAINT and P2=ON_CONFLICT_ACTION_ABORT.
- * * OP_HaltIfNull with P1=SQL_CONSTRAINT and P2=ON_CONFLICT_ACTION_ABORT.
- * * OP_FkCounter with P2==0 (immediate foreign key constraint)
- *
- * Then check that the value of Parse.mayAbort is true if an
- * ABORT may be thrown, or false otherwise. Return true if it does
- * match, or false otherwise. This function is intended to be used as
- * part of an assert statement in the compiler. Similar to:
- *
- * assert( sqlVdbeAssertMayAbort(pParse->pVdbe, pParse->mayAbort) );
- */
-int
-sqlVdbeAssertMayAbort(Vdbe * v, int mayAbort)
-{
- int hasAbort = 0;
- int hasFkCounter = 0;
- Op *pOp;
- VdbeOpIter sIter;
- memset(&sIter, 0, sizeof(sIter));
- sIter.v = v;
-
- while ((pOp = opIterNext(&sIter)) != 0) {
- int opcode = pOp->opcode;
- if ((opcode == OP_Halt || opcode == OP_HaltIfNull) &&
- (pOp->p1 & 0xff) == SQL_CONSTRAINT &&
- pOp->p2 == ON_CONFLICT_ACTION_ABORT){
- hasAbort = 1;
- break;
- }
- if (opcode == OP_FkCounter && pOp->p1 == 0 && pOp->p2 == 1) {
- hasFkCounter = 1;
- }
- }
- sqlDbFree(v->db, sIter.apSub);
-
- /* Return true if hasAbort==mayAbort. Or if a malloc failure occurred.
- * If malloc failed, then the while() loop above may not have iterated
- * through all opcodes and hasAbort may be set incorrectly. Return
- * true for this case to prevent the assert() in the callers frame
- * from failing.
- */
- return (v->db->mallocFailed || hasAbort == mayAbort || hasFkCounter);
-}
-#endif /* SQL_DEBUG - the sqlAssertMayAbort() function */
-
/*
* This routine is called after all opcodes have been inserted. It loops
* through all the opcodes and fixes up some details.
--
GitLab