[tarantool-patches] Re: [PATCH v1 3/3] sql: make SQL_TARANTOOL_ERROR the only errcode of OP_Halt

["n.pettik" <korablev@tarantool.org>]

'make ... the only errcode of OP_Halt’ ->
make … be the only ...

> On 12 Apr 2019, at 15:34, imeevma@tarantool.org wrote:
> 
> After this patch, the only error code that the OP_Halt opcode
> will work with is SQL_TARANTOOL_ERROR.

So, why do we need it at all now? Let’s use simple flag
is_aborted like in parser.

> 
> Part of #4074
> ---
> src/box/sql/build.c         | 20 --------------------
> src/box/sql/expr.c          |  7 ++++---
> src/box/sql/fk_constraint.c |  7 +++----
> src/box/sql/insert.c        | 29 ++++++++++++++---------------
> src/box/sql/sqlInt.h        |  1 -
> src/box/sql/vdbe.c          | 23 ++++-------------------
> 6 files changed, 25 insertions(+), 62 deletions(-)
> 
> 
> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
> index eb08f7e..ba41837 100644
> --- a/src/box/sql/expr.c
> +++ b/src/box/sql/expr.c
> @@ -4409,9 +4409,10 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
> 					  ON_CONFLICT_ACTION_IGNORE, 0,
> 					  pExpr->u.zToken, 0);
> 		} else {
> -			sqlHaltConstraint(pParse, SQL_CONSTRAINT_TRIGGER,
> -					      pExpr->on_conflict_action,
> -					      pExpr->u.zToken, 0, 0);
> +			sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR,
> +				      pExpr->on_conflict_action, 0,
> +				      pExpr->u.zToken, 0);
> +			sqlVdbeChangeP5(v, ER_SQL_EXECUTE);

Are there any other options for P5 argument,
except for ER_SQL_EXECUTE? If not so, let’s always
assume it is ER_SQL_EXECUTE and avoid passing
extra argument.

> 
> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> index 3d20aa9..a9c3f68 100644
> --- a/src/box/sql/vdbe.c
> +++ b/src/box/sql/vdbe.c
> @@ -1069,27 +1069,12 @@ case OP_Halt: {
> 	p->errorAction = (u8)pOp->p2;
> 	p->pc = pcx;
> 	if (p->rc) {
> -		if (p->rc == SQL_TARANTOOL_ERROR) {
> -			if (pOp->p4.z == NULL) {
> -				assert(! diag_is_empty(diag_get()));
> -			} else {
> -				diag_set(ClientError, pOp->p5, pOp->p4.z);
> -			}
> -		} else if (pOp->p5 != 0) {
> -			static const char * const azType[] = { "NOT NULL", "UNIQUE", "CHECK",
> -							       "FOREIGN KEY" };
> -			testcase( pOp->p5==1);
> -			testcase( pOp->p5==2);
> -			testcase( pOp->p5==3);
> -			testcase( pOp->p5==4);
> -			sqlVdbeError(p, "%s constraint failed", azType[pOp->p5-1]);
> -			if (pOp->p4.z) {
> -				p->zErrMsg = sqlMPrintf(db, "%z: %s", p->zErrMsg, pOp->p4.z);
> -			}
> +		assert(p->rc == SQL_TARANTOOL_ERROR);
> +		if (pOp->p4.z == NULL) {
> +			assert(! diag_is_empty(diag_get()));
> 		} else {
> -			sqlVdbeError(p, "%s", pOp->p4.z);
> +			diag_set(ClientError, pOp->p5, pOp->p4.z);
> 		}

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index a9c3f688e..673d6a73f 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1070,11 +1070,9 @@ case OP_Halt: {
        p->pc = pcx;
        if (p->rc) {
                assert(p->rc == SQL_TARANTOOL_ERROR);
-               if (pOp->p4.z == NULL) {
-                       assert(! diag_is_empty(diag_get()));
-               } else {
+               if (pOp->p4.z != NULL)
                        diag_set(ClientError, pOp->p5, pOp->p4.z);
-               }
+               assert(! diag_is_empty(diag_get()));
        }
        rc = sqlVdbeHalt(p);
        assert(rc==SQL_BUSY || rc==SQL_OK || rc==SQL_ERROR);