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 C60402F17E for ; Sat, 25 May 2019 05:45:20 -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 JczqVEhSmQxr for ; Sat, 25 May 2019 05:45:20 -0400 (EDT) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 505D62EE42 for ; Sat, 25 May 2019 05:45:19 -0400 (EDT) Date: Sat, 25 May 2019 12:45:16 +0300 From: Mergen Imeev Subject: [tarantool-patches] Re: [PATCH v1 12/12] sql: use diag_set() to set an error in SQL functions Message-ID: <20190525094516.GA15670@tarantool.org> References: <5af3532d78b6d509d8e76ff97e45a6e3b9eb2369.1557056617.git.imeevma@gmail.com> <80806CED-90B4-402B-9D0B-BDF141C7CDF0@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <80806CED-90B4-402B-9D0B-BDF141C7CDF0@tarantool.org> 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: "n.pettik" Cc: tarantool-patches@freelists.org 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); ]], { -- - 1, "Failed to execute SQL statement: integer overflow" + 1, "Failed to execute SQL statement: integer is overflowed" -- })