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

[n.pettik]

> On 5 May 2019, at 15:17, imeevma at tarantool.org wrote:
> 
> Currently, in OP_Halt, you can get a SQL error other than
> SQL_TARANTOOL_ERROR, for example, the SQL_CONSTRAINT error. After
> this patch, all errors going through OP_Halt will have SQL error
> code SQL_TARANTOOL_ERROR and have diag set.
> 
> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> index 9f0d760..85cec85 100644
> --- a/src/box/sql/vdbe.c
> +++ b/src/box/sql/vdbe.c
> @@ -1031,25 +1031,10 @@ 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)
> -				diag_set(ClientError, pOp->p5, pOp->p4.z);
> -			assert(! diag_is_empty(diag_get()));
> -		} 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);
> -			}
> -		} else {
> -			sqlVdbeError(p, "%s", pOp->p4.z);
> -		}
> -		sql_log(pOp->p1, "abort at %d in [%s]: %s", pcx, p->zSql, p->zErrMsg);
> +		assert(p->rc == SQL_TARANTOOL_ERROR);

Still don’t understand why can’t we use only 0/-1 for rc.

> +		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);
> -- 
> 2.7.4
> 
>