[tarantool-patches] Re: [PATCH v1 12/12] sql: use diag_set() to set an error in SQL functions
Imeev Mergen
imeevma at tarantool.org
Sat May 25 13:36:32 MSK 2019
Fixed commit-message:
After this patch, all errors in the SQL functions will be set
using diag_set().
Part of#4074 <https://github.com/tarantool/tarantool/issues/4074>
On 5/25/19 12:45 PM, Mergen Imeev wrote:
> I moved this patch to the position before "sql: set errors
> in VDBE using diag_set()". New patch below.
>
> On Wed, May 15, 2019 at 05:12:09PM +0300, n.pettik wrote:
>>> After this patch, all errors in the SQL functions will be set
>>> using diag_set().
>>>
>>> Closes #4074
>>> ---
>>> src/box/lua/lua_sql.c | 13 +++--
>>> src/box/sql/analyze.c | 6 +--
>>> src/box/sql/func.c | 104 ++++++++++++++++++----------------------
>>> src/box/sql/sqlInt.h | 13 -----
>>> src/box/sql/vdbe.c | 34 +++----------
>>> src/box/sql/vdbeInt.h | 28 ++---------
>>> src/box/sql/vdbeapi.c | 129 +++++---------------------------------------------
>>> src/box/sql/vdbeaux.c | 46 ------------------
>>> src/box/sql/vdbemem.c | 9 ++--
>>> 9 files changed, 84 insertions(+), 298 deletions(-)
>>>
>>> diff --git a/src/box/lua/lua_sql.c b/src/box/lua/lua_sql.c
>>> index d28045a..afe4732 100644
>>> --- a/src/box/lua/lua_sql.c
>>> +++ b/src/box/lua/lua_sql.c
>>> @@ -77,13 +77,15 @@ lua_sql_call(sql_context *pCtx, int nVal, sql_value **apVal) {
>>> lua_pushboolean(L, sql_value_boolean(param));
>>> break;
>>> default:
>>> - sql_result_error(pCtx, "Unsupported type passed "
>>> - "to Lua", -1);
>> Please, remove sql_result_error at all: I grepped several
>> usages among dead code.
>>
> Fixed.
>
>>> + diag_set(ClientError, ER_SQL_EXECUTE, "Unsupported "\
>>> + "type passed to Lua");
>>> + pCtx->is_error = true;
>>> goto error;
>>> }
>>>
>>> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
>>> index bb7405e..16c02f1 100644
>>> --- a/src/box/sql/func.c
>>> +++ b/src/box/sql/func.c
>>> @@ -181,13 +181,9 @@ absFunc(sql_context * context, int argc, sql_value ** argv)
>>> i64 iVal = sql_value_int64(argv[0]);
>>> if (iVal < 0) {
>>> if (iVal == SMALLEST_INT64) {
>>> - /* IMP: R-31676-45509 If X is the integer -9223372036854775808
>>> - * then abs(X) throws an integer overflow error since there is no
>>> - * equivalent positive 64-bit two complement value.
>>> - */
>>> - sql_result_error(context,
>>> - "integer overflow",
>>> - -1);
>>> + diag_set(ClientError, ER_SQL_EXECUTE,
>>> + "integer overflow”);
>> -> integer is overflowed.
>>
> Fixed.
>
>>> + context->is_error = true;
>>> return;
>>> }
>>> iVal = -iVal;
>>>
>>>
>>>
>>> @@ -591,11 +577,11 @@ case_type##ICUFunc(sql_context *context, int argc, sql_value **argv) \
>>> * does not invalidate the _text() pointer. \
>>> */ \
>>> assert(z2 == (char *)sql_value_text(argv[0])); \
>>> - if (!z2) \
>>> + if (z2 == NULL) \
>>> return; \
>> Redundant diff.
>>
> Fixed.
>
>>> src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
>>> index 6aadca2..70a0bab 100644
>>> --- a/src/box/sql/vdbeInt.h
>>> +++ b/src/box/sql/vdbeInt.h
>>>
>>> @@ -300,21 +296,6 @@ mem_apply_numeric_type(struct Mem *record);
>>> #endif
>>>
>>>
>>> -/*
>>> * The "context" argument for an installable function. A pointer to an
>>> * instance of this structure is the first argument to the routines used
>>> * implement the SQL functions.
>>> @@ -333,9 +314,12 @@ struct sql_context {
>>> Mem *pMem; /* Memory cell used to store aggregate context */
>>> Vdbe *pVdbe; /* The VM that owns this context */
>>> int iOp; /* Instruction number of OP_Function */
>>> - int isError; /* Error code returned by the function. */
>>> + /*
>>> + * True, if an error occurred during the execution of the
>>> + * function.
>>> + */
>>> + bool is_error;
>> I’d better use is_abotred name.
>>
> FIxed. Renamed to is_aborted.
>
>>> diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
>>> index f73ea0a..ce0c641 100644
>>> --- a/src/box/sql/vdbemem.c
>>> +++ b/src/box/sql/vdbemem.c
>>> @@ -321,7 +321,6 @@ sqlVdbeMemStringify(Mem * pMem, u8 bForce)
>>> int
>>> sqlVdbeMemFinalize(Mem * pMem, FuncDef * pFunc)
>>> {
>>> - int rc = SQL_OK;
>>> if (ALWAYS(pFunc && pFunc->xFinalize)) {
>>> sql_context ctx;
>>> Mem t;
>>> @@ -338,9 +337,9 @@ sqlVdbeMemFinalize(Mem * pMem, FuncDef * pFunc)
>>> if (pMem->szMalloc > 0)
>>> sqlDbFree(pMem->db, pMem->zMalloc);
>>> memcpy(pMem, &t, sizeof(t));
>>> - rc = ctx.isError;
>>> + return ctx.is_error ? SQL_TARANTOOL_ERROR : SQL_OK;
>>> }
>>> - return rc;
>>> + return SQL_OK;
>> Return 0/-1
>>
> It is fixed in one of following patches.
>
>
> New patch:
>
> From bb66ac8360ee312cf8d2047143139bb46441ddf0 Mon Sep 17 00:00:00 2001
> Date: Sat, 27 Apr 2019 21:17:19 +0300
> Subject: [PATCH] sql: use diag_set() to set an error in SQL functions
>
> After this patch, all errors in the SQL functions will be set
> using diag_set().
>
> Closes #4074
>
> diff --git a/src/box/lua/lua_sql.c b/src/box/lua/lua_sql.c
> index 36b75ff..529ee59 100644
> --- a/src/box/lua/lua_sql.c
> +++ b/src/box/lua/lua_sql.c
> @@ -77,13 +77,15 @@ lua_sql_call(sql_context *pCtx, int nVal, sql_value **apVal) {
> lua_pushboolean(L, sql_value_boolean(param));
> break;
> default:
> - sql_result_error(pCtx, "Unsupported type passed "
> - "to Lua", -1);
> + diag_set(ClientError, ER_SQL_EXECUTE, "Unsupported "\
> + "type passed to Lua");
> + pCtx->is_aborted = true;
> goto error;
> }
> }
> if (lua_pcall(L, lua_gettop(L) - 1, 1, 0) != 0){
> - sql_result_error(pCtx, lua_tostring(L, -1), -1);
> + diag_set(ClientError, ER_SQL_EXECUTE, lua_tostring(L, -1));
> + pCtx->is_aborted = true;
> goto error;
> }
> switch(lua_type(L, -1)) {
> @@ -101,8 +103,9 @@ lua_sql_call(sql_context *pCtx, int nVal, sql_value **apVal) {
> sql_result_null(pCtx);
> break;
> default:
> - sql_result_error(pCtx, "Unsupported type returned from Lua",
> - -1);
> + diag_set(ClientError, ER_SQL_EXECUTE, "Unsupported type "\
> + "passed from Lua");
> + pCtx->is_aborted = true;
> goto error;
> }
> error:
> diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
> index d2d29c5..9795429 100644
> --- a/src/box/sql/analyze.c
> +++ b/src/box/sql/analyze.c
> @@ -298,7 +298,7 @@ statInit(sql_context * context, int argc, sql_value ** argv)
> db = sql_context_db_handle(context);
> p = sqlDbMallocZero(db, n);
> if (p == 0) {
> - sql_result_error_nomem(context);
> + context->is_aborted = true;
> return;
> }
>
> @@ -669,7 +669,7 @@ statGet(sql_context * context, int argc, sql_value ** argv)
>
> char *zRet = sqlMallocZero((p->nKeyCol + 1) * 25);
> if (zRet == 0) {
> - sql_result_error_nomem(context);
> + context->is_aborted = true;
> return;
> }
>
> @@ -715,7 +715,7 @@ statGet(sql_context * context, int argc, sql_value ** argv)
>
> char *zRet = sqlMallocZero(p->nCol * 25);
> if (zRet == 0) {
> - sql_result_error_nomem(context);
> + context->is_aborted = true;
> } else {
> int i;
> char *z = zRet;
> diff --git a/src/box/sql/date.c b/src/box/sql/date.c
> index 07a57ab..6d3a2b0 100644
> --- a/src/box/sql/date.c
> +++ b/src/box/sql/date.c
> @@ -617,7 +617,8 @@ localtimeOffset(DateTime * p, /* Date at which to calculate offset */
> computeJD(&x);
> t = (time_t) (x.iJD / 1000 - 21086676 * (i64) 10000);
> if (osLocaltime(&t, &sLocal)) {
> - sql_result_error(pCtx, "local time unavailable", -1);
> + diag_set(ClientError, ER_SQL_EXECUTE, "local time unavailable");
> + pCtx->is_aborted = true;
> *pRc = SQL_ERROR;
> return 0;
> }
> @@ -1108,12 +1109,13 @@ strftimeFunc(sql_context * context, int argc, sql_value ** argv)
> if (n < sizeof(zBuf)) {
> z = zBuf;
> } else if (n > (u64) db->aLimit[SQL_LIMIT_LENGTH]) {
> - sql_result_error_toobig(context);
> + diag_set(ClientError, ER_SQL_EXECUTE, "string or blob too big");
> + context->is_aborted = true;
> return;
> } else {
> z = sqlDbMallocRawNN(db, (int)n);
> if (z == 0) {
> - sql_result_error_nomem(context);
> + context->is_aborted = true;
> return;
> }
> }
> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> index bb7405e..593aa94 100644
> --- a/src/box/sql/func.c
> +++ b/src/box/sql/func.c
> @@ -181,13 +181,9 @@ absFunc(sql_context * context, int argc, sql_value ** argv)
> i64 iVal = sql_value_int64(argv[0]);
> if (iVal < 0) {
> if (iVal == SMALLEST_INT64) {
> - /* IMP: R-31676-45509 If X is the integer -9223372036854775808
> - * then abs(X) throws an integer overflow error since there is no
> - * equivalent positive 64-bit two complement value.
> - */
> - sql_result_error(context,
> - "integer overflow",
> - -1);
> + diag_set(ClientError, ER_SQL_EXECUTE,
> + "integer is overflowed");
> + context->is_aborted = true;
> return;
> }
> iVal = -iVal;
> @@ -203,8 +199,7 @@ absFunc(sql_context * context, int argc, sql_value ** argv)
> case MP_BOOL: {
> diag_set(ClientError, ER_INCONSISTENT_TYPES, "number",
> "boolean");
> - context->isError = SQL_TARANTOOL_ERROR;
> - context->fErrorOrAux = 1;
> + context->is_aborted = true;
> return;
> }
> default:{
> @@ -256,8 +251,7 @@ position_func(struct sql_context *context, int argc, struct Mem **argv)
> if (inconsistent_type_arg != NULL) {
> diag_set(ClientError, ER_INCONSISTENT_TYPES, "TEXT or BLOB",
> mem_type_to_str(inconsistent_type_arg));
> - context->isError = SQL_TARANTOOL_ERROR;
> - context->fErrorOrAux = 1;
> + context->is_aborted = true;
> return;
> }
> /*
> @@ -267,8 +261,7 @@ position_func(struct sql_context *context, int argc, struct Mem **argv)
> if (haystack_type != needle_type) {
> diag_set(ClientError, ER_INCONSISTENT_TYPES,
> mem_type_to_str(needle), mem_type_to_str(haystack));
> - context->isError = SQL_TARANTOOL_ERROR;
> - context->fErrorOrAux = 1;
> + context->is_aborted = true;
> return;
> }
>
> @@ -513,7 +506,6 @@ roundFunc(sql_context * context, int argc, sql_value ** argv)
> {
> int n = 0;
> double r;
> - char *zBuf;
> assert(argc == 1 || argc == 2);
> if (argc == 2) {
> if (sql_value_is_null(argv[1]))
> @@ -534,23 +526,17 @@ roundFunc(sql_context * context, int argc, sql_value ** argv)
> } else if (n == 0 && r < 0 && (-r) < LARGEST_INT64 - 1) {
> r = -(double)((sql_int64) ((-r) + 0.5));
> } else {
> - zBuf = sql_mprintf("%.*f", n, r);
> - if (zBuf == 0) {
> - sql_result_error_nomem(context);
> - return;
> - }
> - sqlAtoF(zBuf, &r, sqlStrlen30(zBuf));
> - sql_free(zBuf);
> + const char *rounded_value = tt_sprintf("%.*f", n, r);
> + sqlAtoF(rounded_value, &r, sqlStrlen30(rounded_value));
> }
> sql_result_double(context, r);
> }
>
> /*
> * Allocate nByte bytes of space using sqlMalloc(). If the
> - * allocation fails, call sql_result_error_nomem() to notify
> - * the database handle that malloc() has failed and return NULL.
> - * If nByte is larger than the maximum string or blob length, then
> - * raise an SQL_TOOBIG exception and return NULL.
> + * allocation fails, return NULL. If nByte is larger than the
> + * maximum string or blob length, then raise an error and return
> + * NULL.
> */
> static void *
> contextMalloc(sql_context * context, i64 nByte)
> @@ -561,13 +547,13 @@ contextMalloc(sql_context * context, i64 nByte)
> testcase(nByte == db->aLimit[SQL_LIMIT_LENGTH]);
> testcase(nByte == db->aLimit[SQL_LIMIT_LENGTH] + 1);
> if (nByte > db->aLimit[SQL_LIMIT_LENGTH]) {
> - sql_result_error_toobig(context);
> + diag_set(ClientError, ER_SQL_EXECUTE, "string or blob too big");
> + context->is_aborted = true;
> z = 0;
> } else {
> z = sqlMalloc(nByte);
> - if (!z) {
> - sql_result_error_nomem(context);
> - }
> + if (z == NULL)
> + context->is_aborted = true;
> }
> return z;
> }
> @@ -594,8 +580,8 @@ case_type##ICUFunc(sql_context *context, int argc, sql_value **argv) \
> if (!z2) \
> return; \
> z1 = contextMalloc(context, ((i64) n) + 1); \
> - if (!z1) { \
> - sql_result_error_nomem(context); \
> + if (z1 == NULL) { \
> + context->is_aborted = true; \
> return; \
> } \
> UErrorCode status = U_ZERO_ERROR; \
> @@ -612,8 +598,8 @@ case_type##ICUFunc(sql_context *context, int argc, sql_value **argv) \
> status = U_ZERO_ERROR; \
> sql_free(z1); \
> z1 = contextMalloc(context, ((i64) len) + 1); \
> - if (!z1) { \
> - sql_result_error_nomem(context); \
> + if (z1 == NULL) { \
> + context->is_aborted = true; \
> return; \
> } \
> ucasemap_utf8To##case_type(case_map, z1, len, z2, n, &status); \
> @@ -919,8 +905,7 @@ likeFunc(sql_context *context, int argc, sql_value **argv)
> mem_type_to_str(argv[1]);
> diag_set(ClientError, ER_INCONSISTENT_TYPES, "TEXT",
> inconsistent_type);
> - context->fErrorOrAux = 1;
> - context->isError = SQL_TARANTOOL_ERROR;
> + context->is_aborted = true;
> return;
> }
> const char *zB = (const char *) sql_value_text(argv[0]);
> @@ -937,8 +922,9 @@ likeFunc(sql_context *context, int argc, sql_value **argv)
> testcase(nPat == db->aLimit[SQL_LIMIT_LIKE_PATTERN_LENGTH]);
> testcase(nPat == db->aLimit[SQL_LIMIT_LIKE_PATTERN_LENGTH] + 1);
> if (nPat > db->aLimit[SQL_LIMIT_LIKE_PATTERN_LENGTH]) {
> - sql_result_error(context,
> - "LIKE pattern is too complex", -1);
> + diag_set(ClientError, ER_SQL_EXECUTE, "LIKE pattern is too "\
> + "complex");
> + context->is_aborted = true;
> return;
> }
> /* Encoding did not change */
> @@ -953,10 +939,10 @@ likeFunc(sql_context *context, int argc, sql_value **argv)
> const unsigned char *zEsc = sql_value_text(argv[2]);
> if (zEsc == 0)
> return;
> - const char *const err_msg =
> - "ESCAPE expression must be a single character";
> if (sql_utf8_char_count(zEsc, sql_value_bytes(argv[2])) != 1) {
> - sql_result_error(context, err_msg, -1);
> + diag_set(ClientError, ER_SQL_EXECUTE, "ESCAPE "\
> + "expression must be a single character");
> + context->is_aborted = true;
> return;
> }
> escape = sqlUtf8Read(&zEsc);
> @@ -967,9 +953,9 @@ likeFunc(sql_context *context, int argc, sql_value **argv)
> res = sql_utf8_pattern_compare(zB, zA, zB_end, zA_end,
> is_like_ci, escape);
> if (res == INVALID_PATTERN) {
> - const char *const err_msg =
> - "LIKE pattern can only contain UTF-8 characters";
> - sql_result_error(context, err_msg, -1);
> + diag_set(ClientError, ER_SQL_EXECUTE, "LIKE pattern can only "\
> + "contain UTF-8 characters");
> + context->is_aborted = true;
> return;
> }
> sql_result_bool(context, res == MATCH);
> @@ -1136,8 +1122,8 @@ charFunc(sql_context * context, int argc, sql_value ** argv)
> unsigned char *z, *zOut;
> int i;
> zOut = z = sql_malloc64(argc * 4 + 1);
> - if (z == 0) {
> - sql_result_error_nomem(context);
> + if (z == NULL) {
> + context->is_aborted = true;
> return;
> }
> for (i = 0; i < argc; i++) {
> @@ -1200,15 +1186,14 @@ static void
> zeroblobFunc(sql_context * context, int argc, sql_value ** argv)
> {
> i64 n;
> - int rc;
> assert(argc == 1);
> UNUSED_PARAMETER(argc);
> n = sql_value_int64(argv[0]);
> if (n < 0)
> n = 0;
> - rc = sql_result_zeroblob64(context, n); /* IMP: R-00293-64994 */
> - if (rc) {
> - sql_result_error_code(context, rc);
> + if (sql_result_zeroblob64(context, n) != SQL_OK) {
> + diag_set(ClientError, ER_SQL_EXECUTE, "string or blob too big");
> + context->is_aborted = true;
> }
> }
>
> @@ -1275,14 +1260,16 @@ replaceFunc(sql_context * context, int argc, sql_value ** argv)
> testcase(nOut - 1 == db->aLimit[SQL_LIMIT_LENGTH]);
> testcase(nOut - 2 == db->aLimit[SQL_LIMIT_LENGTH]);
> if (nOut - 1 > db->aLimit[SQL_LIMIT_LENGTH]) {
> - sql_result_error_toobig(context);
> + diag_set(ClientError, ER_SQL_EXECUTE, "string "\
> + "or blob too big");
> + context->is_aborted = true;
> sql_free(zOut);
> return;
> }
> zOld = zOut;
> zOut = sql_realloc64(zOut, (int)nOut);
> if (zOut == 0) {
> - sql_result_error_nomem(context);
> + context->is_aborted = true;
> sql_free(zOld);
> return;
> }
> @@ -1605,8 +1592,7 @@ sum_step(struct sql_context *context, int argc, sql_value **argv)
> if (mem_apply_numeric_type(argv[0]) != 0) {
> diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
> sql_value_text(argv[0]), "number");
> - context->fErrorOrAux = 1;
> - context->isError = SQL_TARANTOOL_ERROR;
> + context->is_aborted = true;
> return;
> }
> type = sql_value_type(argv[0]);
> @@ -1632,7 +1618,9 @@ sumFinalize(sql_context * context)
> p = sql_aggregate_context(context, 0);
> if (p && p->cnt > 0) {
> if (p->overflow) {
> - sql_result_error(context, "integer overflow", -1);
> + diag_set(ClientError, ER_SQL_EXECUTE, "integer "\
> + "overflow");
> + context->is_aborted = true;
> } else if (p->approx) {
> sql_result_double(context, p->rSum);
> } else {
> @@ -1789,9 +1777,11 @@ groupConcatFinalize(sql_context * context)
> pAccum = sql_aggregate_context(context, 0);
> if (pAccum) {
> if (pAccum->accError == STRACCUM_TOOBIG) {
> - sql_result_error_toobig(context);
> + diag_set(ClientError, ER_SQL_EXECUTE, "string or blob "\
> + "too big");
> + context->is_aborted = true;
> } else if (pAccum->accError == STRACCUM_NOMEM) {
> - sql_result_error_nomem(context);
> + context->is_aborted = true;
> } else {
> sql_result_text(context,
> sqlStrAccumFinish(pAccum),
> diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
> index 3981fbf..9c0659b 100644
> --- a/src/box/sql/sqlInt.h
> +++ b/src/box/sql/sqlInt.h
> @@ -476,19 +476,6 @@ void
> sql_result_double(sql_context *, double);
>
> void
> -sql_result_error(sql_context *, const char *,
> - int);
> -
> -void
> -sql_result_error_toobig(sql_context *);
> -
> -void
> -sql_result_error_nomem(sql_context *);
> -
> -void
> -sql_result_error_code(sql_context *, int);
> -
> -void
> sql_result_int(sql_context *, int);
>
> void
> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> index 1559f01..7d85959 100644
> --- a/src/box/sql/vdbe.c
> +++ b/src/box/sql/vdbe.c
> @@ -1717,17 +1717,13 @@ case OP_Function: {
> }
> #endif
> MemSetTypeFlag(pCtx->pOut, MEM_Null);
> - pCtx->fErrorOrAux = 0;
> + pCtx->is_aborted = false;
> (*pCtx->pFunc->xSFunc)(pCtx, pCtx->argc, pCtx->argv);/* IMP: R-24505-23230 */
>
> /* If the function returned an error, throw an exception */
> - if (pCtx->fErrorOrAux) {
> - if (pCtx->isError) {
> - sqlVdbeError(p, "%s", sql_value_text(pCtx->pOut));
> - rc = pCtx->isError;
> - }
> - sqlVdbeDeleteAuxData(db, &p->pAuxData, pCtx->iOp, pOp->p1);
> - if (rc) goto abort_due_to_error;
> + if (pCtx->is_aborted) {
> + rc = SQL_TARANTOOL_ERROR;
> + goto abort_due_to_error;
> }
>
> /* Copy the result of the function into register P3 */
> @@ -4894,9 +4890,6 @@ case OP_Program: { /* jump */
> pFrame->pParent = p->pFrame;
> pFrame->nChange = p->nChange;
> pFrame->nDbChange = p->db->nChange;
> - assert(pFrame->pAuxData==0);
> - pFrame->pAuxData = p->pAuxData;
> - p->pAuxData = 0;
> p->nChange = 0;
> p->pFrame = pFrame;
> p->aMem = aMem = VdbeFrameMem(pFrame);
> @@ -5159,16 +5152,13 @@ case OP_AggStep: {
> pMem->n++;
> sqlVdbeMemInit(&t, db, MEM_Null);
> pCtx->pOut = &t;
> - pCtx->fErrorOrAux = 0;
> + pCtx->is_aborted = false;
> pCtx->skipFlag = 0;
> (pCtx->pFunc->xSFunc)(pCtx,pCtx->argc,pCtx->argv); /* IMP: R-24505-23230 */
> - if (pCtx->fErrorOrAux) {
> - if (pCtx->isError) {
> - sqlVdbeError(p, "%s", sql_value_text(&t));
> - rc = pCtx->isError;
> - }
> + if (pCtx->is_aborted) {
> sqlVdbeMemRelease(&t);
> - if (rc) goto abort_due_to_error;
> + rc = SQL_TARANTOOL_ERROR;
> + goto abort_due_to_error;
> } else {
> assert(t.flags==MEM_Null);
> }
> diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
> index a3100e5..ee14510 100644
> --- a/src/box/sql/vdbeInt.h
> +++ b/src/box/sql/vdbeInt.h
> @@ -62,9 +62,6 @@ typedef unsigned Bool;
> /* Opaque type used by code in vdbesort.c */
> typedef struct VdbeSorter VdbeSorter;
>
> -/* Elements of the linked list at Vdbe.pAuxData */
> -typedef struct AuxData AuxData;
> -
> /* Types of VDBE cursors */
> #define CURTYPE_TARANTOOL 0
> #define CURTYPE_SORTER 1
> @@ -161,7 +158,6 @@ struct VdbeFrame {
> Mem *aMem; /* Array of memory cells for parent frame */
> VdbeCursor **apCsr; /* Array of Vdbe cursors for parent frame */
> void *token; /* Copy of SubProgram.token */
> - AuxData *pAuxData; /* Linked list of auxdata allocations */
> int nCursor; /* Number of entries in apCsr */
> int pc; /* Program Counter in parent (calling) frame */
> int nOp; /* Size of aOp array */
> @@ -304,21 +300,6 @@ mem_apply_numeric_type(struct Mem *record);
> #endif
>
> /*
> - * Each auxiliary data pointer stored by a user defined function
> - * implementation calling sql_set_auxdata() is stored in an instance
> - * of this structure. All such structures associated with a single VM
> - * are stored in a linked list headed at Vdbe.pAuxData. All are destroyed
> - * when the VM is halted (if not before).
> - */
> -struct AuxData {
> - int iOp; /* Instruction number of OP_Function opcode */
> - int iArg; /* Index of function argument. */
> - void *pAux; /* Aux data pointer */
> - void (*xDelete) (void *); /* Destructor for the aux data */
> - AuxData *pNext; /* Next element in list */
> -};
> -
> -/*
> * The "context" argument for an installable function. A pointer to an
> * instance of this structure is the first argument to the routines used
> * implement the SQL functions.
> @@ -337,9 +318,12 @@ struct sql_context {
> Mem *pMem; /* Memory cell used to store aggregate context */
> Vdbe *pVdbe; /* The VM that owns this context */
> int iOp; /* Instruction number of OP_Function */
> - int isError; /* Error code returned by the function. */
> + /*
> + * True, if an error occurred during the execution of the
> + * function.
> + */
> + bool is_aborted;
> u8 skipFlag; /* Skip accumulator loading if true */
> - u8 fErrorOrAux; /* isError!=0 or pVdbe->pAuxData modified */
> u8 argc; /* Number of arguments */
> sql_value *argv[1]; /* Argument set */
> };
> @@ -445,7 +429,6 @@ struct Vdbe {
> int nFrame; /* Number of frames in pFrame list */
> u32 expmask; /* Binding to these vars invalidates VM */
> SubProgram *pProgram; /* Linked list of all sub-programs used by VM */
> - AuxData *pAuxData; /* Linked list of auxdata allocations */
> /* Anonymous savepoint for aborts only */
> Savepoint *anonymous_savepoint;
> #ifdef SQL_ENABLE_STMT_SCANSTATUS
> @@ -478,7 +461,6 @@ u32 sqlVdbeSerialTypeLen(u32);
> u32 sqlVdbeSerialType(Mem *, int, u32 *);
> u32 sqlVdbeSerialPut(unsigned char *, Mem *, u32);
> u32 sqlVdbeSerialGet(const unsigned char *, u32, Mem *);
> -void sqlVdbeDeleteAuxData(sql *, AuxData **, int, int);
>
> int sqlVdbeExec(Vdbe *);
> int sqlVdbeList(Vdbe *);
> diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
> index 597d37e..673ccd1 100644
> --- a/src/box/sql/vdbeapi.c
> +++ b/src/box/sql/vdbeapi.c
> @@ -321,7 +321,8 @@ setResultStrOrError(sql_context * pCtx, /* Function context */
> )
> {
> if (sqlVdbeMemSetStr(pCtx->pOut, z, n,1, xDel) == SQL_TOOBIG) {
> - sql_result_error_toobig(pCtx);
> + diag_set(ClientError, ER_SQL_EXECUTE, "string or blob too big");
> + pCtx->is_aborted = true;
> }
> }
>
> @@ -339,8 +340,10 @@ invokeValueDestructor(const void *p, /* Value to destroy */
> } else {
> xDel((void *)p);
> }
> - if (pCtx)
> - sql_result_error_toobig(pCtx);
> + if (pCtx) {
> + diag_set(ClientError, ER_SQL_EXECUTE, "string or blob too big");
> + pCtx->is_aborted = true;
> + }
> return SQL_TOOBIG;
> }
>
> @@ -351,7 +354,8 @@ sql_result_blob(sql_context * pCtx,
> {
> assert(n >= 0);
> if (sqlVdbeMemSetStr(pCtx->pOut, z, n,0, xDel) == SQL_TOOBIG) {
> - sql_result_error_toobig(pCtx);
> + diag_set(ClientError, ER_SQL_EXECUTE, "string or blob too big");
> + pCtx->is_aborted = true;
> }
> }
>
> @@ -375,14 +379,6 @@ sql_result_double(sql_context * pCtx, double rVal)
> }
>
> void
> -sql_result_error(sql_context * pCtx, const char *z, int n)
> -{
> - pCtx->isError = SQL_ERROR;
> - pCtx->fErrorOrAux = 1;
> - sqlVdbeMemSetStr(pCtx->pOut, z, n, 1, SQL_TRANSIENT);
> -}
> -
> -void
> sql_result_int(sql_context * pCtx, int iVal)
> {
> sqlVdbeMemSetInt64(pCtx->pOut, (i64) iVal);
> @@ -451,37 +447,6 @@ sql_result_zeroblob64(sql_context * pCtx, u64 n)
> return SQL_OK;
> }
>
> -void
> -sql_result_error_code(sql_context * pCtx, int errCode)
> -{
> - pCtx->isError = errCode;
> - pCtx->fErrorOrAux = 1;
> - if (pCtx->pOut->flags & MEM_Null) {
> - sqlVdbeMemSetStr(pCtx->pOut, sqlErrStr(errCode), -1, 1,
> - SQL_STATIC);
> - }
> -}
> -
> -/* Force an SQL_TOOBIG error. */
> -void
> -sql_result_error_toobig(sql_context * pCtx)
> -{
> - pCtx->isError = SQL_TOOBIG;
> - pCtx->fErrorOrAux = 1;
> - sqlVdbeMemSetStr(pCtx->pOut, "string or blob too big", -1, 1,
> - SQL_STATIC);
> -}
> -
> -/* An SQL_NOMEM error. */
> -void
> -sql_result_error_nomem(sql_context * pCtx)
> -{
> - sqlVdbeMemSetNull(pCtx->pOut);
> - pCtx->isError = SQL_NOMEM;
> - pCtx->fErrorOrAux = 1;
> - sqlOomFault(pCtx->pOut->db);
> -}
> -
> /*
> * Execute the statement pStmt, either until a row of data is ready, the
> * statement is completely executed or an error occurs.
> @@ -712,13 +677,10 @@ sqlInvalidFunction(sql_context * context, /* The function calling context */
> )
> {
> const char *zName = context->pFunc->zName;
> - char *zErr;
> UNUSED_PARAMETER2(NotUsed, NotUsed2);
> - zErr =
> - sql_mprintf
> - ("unable to use function %s in the requested context", zName);
> - sql_result_error(context, zErr, -1);
> - sql_free(zErr);
> + const char *err = "unable to use function %s in the requested context";
> + diag_set(ClientError, ER_SQL_EXECUTE, tt_sprintf(err, zName));
> + context->is_aborted = true;
> }
>
> /*
> @@ -762,75 +724,6 @@ sql_aggregate_context(sql_context * p, int nByte)
> }
>
> /*
> - * Return the auxiliary data pointer, if any, for the iArg'th argument to
> - * the user-function defined by pCtx.
> - */
> -void *
> -sql_get_auxdata(sql_context * pCtx, int iArg)
> -{
> - AuxData *pAuxData;
> -
> - if (pCtx->pVdbe == 0)
> - return 0;
> -
> - for (pAuxData = pCtx->pVdbe->pAuxData; pAuxData;
> - pAuxData = pAuxData->pNext) {
> - if (pAuxData->iOp == pCtx->iOp && pAuxData->iArg == iArg)
> - break;
> - }
> -
> - return (pAuxData ? pAuxData->pAux : 0);
> -}
> -
> -/*
> - * Set the auxiliary data pointer and delete function, for the iArg'th
> - * argument to the user-function defined by pCtx. Any previous value is
> - * deleted by calling the delete function specified when it was set.
> - */
> -void
> -sql_set_auxdata(sql_context * pCtx,
> - int iArg, void *pAux, void (*xDelete) (void *)
> - )
> -{
> - AuxData *pAuxData;
> - Vdbe *pVdbe = pCtx->pVdbe;
> -
> - if (iArg < 0)
> - goto failed;
> - if (pVdbe == 0)
> - goto failed;
> -
> - for (pAuxData = pVdbe->pAuxData; pAuxData; pAuxData = pAuxData->pNext) {
> - if (pAuxData->iOp == pCtx->iOp && pAuxData->iArg == iArg)
> - break;
> - }
> - if (pAuxData == 0) {
> - pAuxData = sqlDbMallocZero(pVdbe->db, sizeof(AuxData));
> - if (!pAuxData)
> - goto failed;
> - pAuxData->iOp = pCtx->iOp;
> - pAuxData->iArg = iArg;
> - pAuxData->pNext = pVdbe->pAuxData;
> - pVdbe->pAuxData = pAuxData;
> - if (pCtx->fErrorOrAux == 0) {
> - pCtx->isError = 0;
> - pCtx->fErrorOrAux = 1;
> - }
> - } else if (pAuxData->xDelete) {
> - pAuxData->xDelete(pAuxData->pAux);
> - }
> -
> - pAuxData->pAux = pAux;
> - pAuxData->xDelete = xDelete;
> - return;
> -
> - failed:
> - if (xDelete) {
> - xDelete(pAux);
> - }
> -}
> -
> -/*
> * Return the number of columns in the result set for the statement pStmt.
> */
> int
> diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
> index 27fa5b2..619b211 100644
> --- a/src/box/sql/vdbeaux.c
> +++ b/src/box/sql/vdbeaux.c
> @@ -1401,7 +1401,6 @@ sqlVdbeFrameDelete(VdbeFrame * p)
> sqlVdbeFreeCursor(p->v, apCsr[i]);
> }
> releaseMemArray(aMem, p->nChildMem);
> - sqlVdbeDeleteAuxData(p->v->db, &p->pAuxData, -1, 0);
> sqlDbFree(p->v->db, p);
> }
>
> @@ -1920,9 +1919,6 @@ sqlVdbeFrameRestore(VdbeFrame * pFrame)
> v->nCursor = pFrame->nCursor;
> v->nChange = pFrame->nChange;
> v->db->nChange = pFrame->nDbChange;
> - sqlVdbeDeleteAuxData(v->db, &v->pAuxData, -1, 0);
> - v->pAuxData = pFrame->pAuxData;
> - pFrame->pAuxData = 0;
> return pFrame->pc;
> }
>
> @@ -1966,11 +1962,6 @@ closeCursorsAndFree(Vdbe * p)
> p->pDelFrame = pDel->pParent;
> sqlVdbeFrameDelete(pDel);
> }
> -
> - /* Delete any auxdata allocations made by the VM */
> - if (p->pAuxData)
> - sqlVdbeDeleteAuxData(p->db, &p->pAuxData, -1, 0);
> - assert(p->pAuxData == 0);
> }
>
> /*
> @@ -2528,43 +2519,6 @@ sqlVdbeFinalize(Vdbe * p)
> }
>
> /*
> - * If parameter iOp is less than zero, then invoke the destructor for
> - * all auxiliary data pointers currently cached by the VM passed as
> - * the first argument.
> - *
> - * Or, if iOp is greater than or equal to zero, then the destructor is
> - * only invoked for those auxiliary data pointers created by the user
> - * function invoked by the OP_Function opcode at instruction iOp of
> - * VM pVdbe, and only then if:
> - *
> - * * the associated function parameter is the 32nd or later (counting
> - * from left to right), or
> - *
> - * * the corresponding bit in argument mask is clear (where the first
> - * function parameter corresponds to bit 0 etc.).
> - */
> -void
> -sqlVdbeDeleteAuxData(sql * db, AuxData ** pp, int iOp, int mask)
> -{
> - while (*pp) {
> - AuxData *pAux = *pp;
> - if ((iOp < 0)
> - || (pAux->iOp == iOp
> - && (pAux->iArg > 31 || !(mask & MASKBIT32(pAux->iArg))))
> - ) {
> - testcase(pAux->iArg == 31);
> - if (pAux->xDelete) {
> - pAux->xDelete(pAux->pAux);
> - }
> - *pp = pAux->pNext;
> - sqlDbFree(db, pAux);
> - } else {
> - pp = &pAux->pNext;
> - }
> - }
> -}
> -
> -/*
> * Free all memory associated with the Vdbe passed as the second argument,
> * except for object itself, which is preserved.
> *
> diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
> index f73ea0a..585dc21 100644
> --- a/src/box/sql/vdbemem.c
> +++ b/src/box/sql/vdbemem.c
> @@ -321,7 +321,6 @@ sqlVdbeMemStringify(Mem * pMem, u8 bForce)
> int
> sqlVdbeMemFinalize(Mem * pMem, FuncDef * pFunc)
> {
> - int rc = SQL_OK;
> if (ALWAYS(pFunc && pFunc->xFinalize)) {
> sql_context ctx;
> Mem t;
> @@ -338,9 +337,9 @@ sqlVdbeMemFinalize(Mem * pMem, FuncDef * pFunc)
> if (pMem->szMalloc > 0)
> sqlDbFree(pMem->db, pMem->zMalloc);
> memcpy(pMem, &t, sizeof(t));
> - rc = ctx.isError;
> + return ctx.is_aborted ? SQL_TARANTOOL_ERROR : SQL_OK;
> }
> - return rc;
> + return SQL_OK;
> }
>
> /*
> @@ -1281,7 +1280,7 @@ valueFromFunction(sql * db, /* The database connection */
> ctx.pOut = pVal;
> ctx.pFunc = pFunc;
> pFunc->xSFunc(&ctx, nVal, apVal);
> - assert(!ctx.isError);
> + assert(!ctx.is_aborted);
> sql_value_apply_type(pVal, type);
> assert(rc == SQL_OK);
>
> @@ -1486,7 +1485,7 @@ recordFunc(sql_context * context, int argc, sql_value ** argv)
> nRet = 1 + nSerial + nVal;
> aRet = sqlDbMallocRawNN(db, nRet);
> if (aRet == 0) {
> - sql_result_error_nomem(context);
> + context->is_aborted = true;
> } else {
> aRet[0] = nSerial + 1;
> putVarint32(&aRet[1], iSerial);
> diff --git a/test/sql-tap/func.test.lua b/test/sql-tap/func.test.lua
> index 09b1cf9..f9044ad 100755
> --- a/test/sql-tap/func.test.lua
> +++ b/test/sql-tap/func.test.lua
> @@ -1730,7 +1730,7 @@ test:do_catchsql_test(
> SELECT abs(-9223372036854775807-1);
> ]], {
> -- <func-18.32>
> - 1, "Failed to execute SQL statement: integer overflow"
> + 1, "Failed to execute SQL statement: integer is overflowed"
> -- </func-18.32>
> })
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20190525/f3fdbb3d/attachment.html>
More information about the Tarantool-patches
mailing list