From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id AA0922C6B0 for ; Fri, 12 Apr 2019 08:34:19 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Me3jGu5I_-v1 for ; Fri, 12 Apr 2019 08:34:19 -0400 (EDT) Received: from smtp3.mail.ru (smtp3.mail.ru [94.100.179.58]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 3760A2C6B6 for ; Fri, 12 Apr 2019 08:34:19 -0400 (EDT) From: imeevma@tarantool.org Subject: [tarantool-patches] [PATCH v1 1/3] sql: remove mayAbort field from struct Parse Date: Fri, 12 Apr 2019 15:34:17 +0300 Message-Id: In-Reply-To: References: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: korablev@tarantool.org Cc: tarantool-patches@freelists.org Currently, the mayAbort field is used only in one place in debug mode and is not used in non-debug mode. This patch removes this field. Part of #4074 --- src/box/sql/build.c | 28 ---------- src/box/sql/expr.c | 2 - src/box/sql/fk_constraint.c | 35 ------------ src/box/sql/insert.c | 2 - src/box/sql/sqlInt.h | 2 - src/box/sql/vdbe.h | 3 -- src/box/sql/vdbeaux.c | 127 -------------------------------------------- 7 files changed, 199 deletions(-) diff --git a/src/box/sql/build.c b/src/box/sql/build.c index a06ba3e..febb2e8 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -152,8 +152,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); @@ -2986,29 +2984,6 @@ sql_set_multi_write(struct Parse *parse_context, bool 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 * and/or current transaction is rolled back. @@ -3024,9 +2999,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 6ff41a5..eb08f7e 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -4403,8 +4403,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 8151c66..7d36edc 100644 --- a/src/box/sql/fk_constraint.c +++ b/src/box/sql/fk_constraint.c @@ -292,8 +292,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); } @@ -634,41 +632,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 c2aac55..f725478 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -899,8 +899,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 b322602..bc8ba49 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -2612,7 +2612,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 */ @@ -3977,7 +3976,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 f9bb96f..6234dff 100644 --- a/src/box/sql/vdbe.h +++ b/src/box/sql/vdbe.h @@ -253,9 +253,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 0cc3c14..c8bc731 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -499,133 +499,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. -- 2.7.4