[tarantool-patches] Re: [PATCH v1 12/12] sql: use diag_set() to set an error in SQL functions
n.pettik
korablev at tarantool.org
Wed May 15 17:12:09 MSK 2019
> 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.
> + 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.
> + 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.
> 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.
> 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
More information about the Tarantool-patches
mailing list