From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 7BD9B6EC5B; Wed, 14 Apr 2021 04:00:20 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 7BD9B6EC5B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1618362020; bh=TC8h0SwKqnHThqAGhVSkVLfmyJOQWww0M4STC56RDe8=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=ftOK+spZVLFpF2lIuzwGC/DU+bg6ZSP3Mrl2lvTJrnLVDnm1mckgcpOEF+I2aElc0 HLNP96flhLxwO3iMIsUALcGOiwiy6FdowtLziWSVdG0mJQiRioM3OghvD9wq/enS6S rXArA0ba/rvfbZgVZOH3v0Ha4KG5ej/IggA9s2R0= Received: from smtp40.i.mail.ru (smtp40.i.mail.ru [94.100.177.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 147806EC5B for ; Wed, 14 Apr 2021 04:00:18 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 147806EC5B Received: by smtp40.i.mail.ru with esmtpa (envelope-from ) id 1lWTtB-0002fC-3F; Wed, 14 Apr 2021 04:00:17 +0300 Date: Wed, 14 Apr 2021 04:00:15 +0300 To: Vladislav Shpilevoy Message-ID: <20210414010015.GA145710@tarantool.org> References: <3bba79b9-7bd4-541d-7287-f8e844e8c6c5@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <3bba79b9-7bd4-541d-7287-f8e844e8c6c5@tarantool.org> X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD92FFCB8E6708E74806859AC5FE18436AEED970E897805ADA4182A05F53808504028133CBB8CFD67733B0D7441D37EE8D0B5CEAD12193B0AB41FB403411EA83830 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE75C385DEB91CEC222EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006377A58729D825217838638F802B75D45FF914D58D5BE9E6BC1A93B80C6DEB9DEE97C6FB206A91F05B265ECD76513C7FE7C13C9FFB1A843899A21114FFCE5AE9CDCD2E47CDBA5A96583C09775C1D3CA48CFCA5A41EBD8A3A0199FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C3ED8438A78DFE0A9E117882F4460429728AD0CFFFB425014E868A13BD56FB6657D81D268191BDAD3DC09775C1D3CA48CF5016067B6CD15BD7BA3038C0950A5D36C8A9BA7A39EFB766EC990983EF5C0329BA3038C0950A5D36D5E8D9A59859A8B6031A4DC555CD247876E601842F6C81A1F004C906525384307823802FF610243D2EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE798228CBAD4AC77F6731C566533BA786AA5CC5B56E945C8DA X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A2368A440D3B0F6089093C9A16E5BC824A2A04A2ABAA09D25379311020FFC8D4ADB200D727D4285EA2582C6ACA3E516441 X-C1DE0DAB: 0D63561A33F958A50C61751CD302FF7374487AE9882C592CFB0435935A4D8A29D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7502E6951B79FF9A3F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34F1257DC9690AEBA16669DE8D994E1B98CA3D312C57AE67ED464E1206AD9111CC985AE60B44B814521D7E09C32AA3244C5E206EF2CAFD15911EF33E6F65EF0D5435DA7DC5AF9B58C0FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojnA7/qPBUIXFbLoMepoMhDA== X-Mailru-Sender: 5C3750E245F362008BC1685FEC6306ED5A6E5106D386FDB53B0D7441D37EE8D06565FCDE7E403FA15105BD0848736F9966FEC6BF5C9C28D97E07721503EA2E00ED97202A5A4E92BF7402F9BA4338D657ED14614B50AE0675 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v5 47/52] sql: introduce mem_get_double() X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Mergen Imeev via Tarantool-patches Reply-To: Mergen Imeev Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Thank you for the review! My answer, diff and new patch below. New patch also includes some changes in vdbe.c - previously I missed some code there, which worked directly with mem->u.r. On Wed, Apr 14, 2021 at 01:04:36AM +0200, Vladislav Shpilevoy wrote: > The same as for the previous getters. The result is ignored in 100% > cases. Therefore it must be called 'unsafe', and return the double > as 'return'. When fails, it can return 0, so as at least the behaviour > is not undefined. The same '0 on error' could be used in the other > getters. Added mem_get_double_unsafe(). Diff: diff --git a/src/box/sql/date.c b/src/box/sql/date.c index dffc23616..dbf460498 100644 --- a/src/box/sql/date.c +++ b/src/box/sql/date.c @@ -929,7 +929,7 @@ isDate(sql_context * context, int argc, sql_value ** argv, DateTime * p) } if ((eType = sql_value_type(argv[0])) == MP_DOUBLE || eType == MP_INT) { - setRawDateNumber(p, sql_value_double(argv[0])); + setRawDateNumber(p, mem_get_double_unsafe(argv[0])); } else { z = sql_value_text(argv[0]); if (!z || parseDateOrTime(context, (char *)z, p)) { diff --git a/src/box/sql/func.c b/src/box/sql/func.c index 97a2f5bb0..dbf899c02 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -229,9 +229,7 @@ absFunc(sql_context * context, int argc, sql_value ** argv) * Abs(X) returns 0.0 if X is a string or blob * that cannot be converted to a numeric value. */ - double rVal; - if (mem_get_double(argv[0], &rVal) != 0) - rVal = 0; + double rVal = mem_get_double_unsafe(argv[0]); if (rVal < 0) rVal = -rVal; sql_result_double(context, rVal); @@ -544,7 +542,7 @@ roundFunc(sql_context * context, int argc, sql_value ** argv) context->is_aborted = true; return; } - mem_get_double(argv[0], &r); + r = mem_get_double_unsafe(argv[0]); /* If Y==0 and X will fit in a 64-bit int, * handle the rounding directly, * otherwise use printf. @@ -1050,7 +1048,7 @@ quoteFunc(sql_context * context, int argc, sql_value ** argv) case MP_DOUBLE:{ double r1, r2; char zBuf[50]; - mem_get_double(argv[0], &r1); + r1 = mem_get_double_unsafe(argv[0]); sql_snprintf(sizeof(zBuf), zBuf, "%!.15g", r1); sqlAtoF(zBuf, &r2, 20); if (r1 != r2) { @@ -1663,9 +1661,7 @@ sum_step(struct sql_context *context, int argc, sql_value **argv) p->overflow = 1; } } else { - double d; - mem_get_double(argv[0], &d); - p->rSum += d; + p->rSum += mem_get_double_unsafe(argv[0]); p->approx = 1; } } diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h index d1c3ec386..d7ffa3444 100644 --- a/src/box/sql/mem.h +++ b/src/box/sql/mem.h @@ -796,6 +796,20 @@ mem_get_uint_unsafe(const struct Mem *mem) int mem_get_double(const struct Mem *mem, double *d); +/** + * Return value of MEM converted to double. This function is not safe since + * there is no proper processing in case mem_get_double() return an error. In + * this case this functions returns 0. + */ +static inline double +mem_get_double_unsafe(const struct Mem *mem) +{ + double d; + if (mem_get_double(mem, &d) != 0) + return 0.; + return d; +} + /** * Simple type to str convertor. It is used to simplify * error reporting. diff --git a/src/box/sql/printf.c b/src/box/sql/printf.c index 97df8af35..2f1948ff8 100644 --- a/src/box/sql/printf.c +++ b/src/box/sql/printf.c @@ -152,9 +152,7 @@ getDoubleArg(PrintfArguments * p) { if (p->nArg <= p->nUsed) return 0.0; - double d; - mem_get_double(p->apArg[p->nUsed++], &d); - return d; + return mem_get_double_unsafe(p->apArg[p->nUsed++]); } static char * diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index f7b6df0d9..1deb8e507 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -2751,24 +2751,22 @@ case OP_SeekGT: { /* jump, in3 */ if (mem_is_str(pIn3)) mem_to_number(pIn3); int64_t i; - if (mem_is_uint(pIn3)) { - i = pIn3->u.u; - is_neg = false; - } else if (mem_is_nint(pIn3)) { - i = pIn3->u.i; - is_neg = true; - } else if (mem_is_double(pIn3)) { - if (pIn3->u.r > (double)INT64_MAX) + if (mem_get_int(pIn3, &i, &is_neg) != 0) { + if (!mem_is_double(pIn3)) { + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, + mem_str(pIn3), "integer"); + goto abort_due_to_error; + } + double d = mem_get_double_unsafe(pIn3); + assert(d >= (double)INT64_MAX || d < (double)INT64_MIN); + /* TODO: add [INT64_MAX, UINT64_MAX) here. */ + if (d > (double)INT64_MAX) i = INT64_MAX; - else if (pIn3->u.r < (double)INT64_MIN) + else if (d < (double)INT64_MIN) i = INT64_MIN; else - i = pIn3->u.r; + i = d; is_neg = i < 0; - } else { - diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - mem_str(pIn3), "integer"); - goto abort_due_to_error; } iKey = i; @@ -2791,7 +2789,7 @@ case OP_SeekGT: { /* jump, in3 */ * (x > 4.9) -> (x >= 5) * (x <= 4.9) -> (x < 5) */ - if (pIn3->u.r<(double)iKey) { + if (mem_get_double_unsafe(pIn3) < (double)iKey) { assert(OP_SeekGE==(OP_SeekGT-1)); assert(OP_SeekLT==(OP_SeekLE-1)); assert((OP_SeekLE & 0x0001)==(OP_SeekGT & 0x0001)); @@ -2801,7 +2799,7 @@ case OP_SeekGT: { /* jump, in3 */ /* If the approximation iKey is smaller than the actual real search * term, substitute <= for < and > for >=. */ - else if (pIn3->u.r>(double)iKey) { + else if (mem_get_double_unsafe(pIn3) > (double)iKey) { assert(OP_SeekLE==(OP_SeekLT+1)); assert(OP_SeekGT==(OP_SeekGE+1)); assert((OP_SeekLT & 0x0001)==(OP_SeekGE & 0x0001)); New patch: commit a007a336371b86c7327db12388f58ee55a511d98 Author: Mergen Imeev Date: Wed Mar 17 14:13:30 2021 +0300 sql: introduce mem_get_double() This patch introduces mem_get_double(). This function is used to receive double value from MEM. If value of MEM is not double, it is converted to double if possible. MEM is not changed. Part of #5818 diff --git a/src/box/sql/date.c b/src/box/sql/date.c index dffc23616..dbf460498 100644 --- a/src/box/sql/date.c +++ b/src/box/sql/date.c @@ -929,7 +929,7 @@ isDate(sql_context * context, int argc, sql_value ** argv, DateTime * p) } if ((eType = sql_value_type(argv[0])) == MP_DOUBLE || eType == MP_INT) { - setRawDateNumber(p, sql_value_double(argv[0])); + setRawDateNumber(p, mem_get_double_unsafe(argv[0])); } else { z = sql_value_text(argv[0]); if (!z || parseDateOrTime(context, (char *)z, p)) { diff --git a/src/box/sql/func.c b/src/box/sql/func.c index 5503a9b16..dbf899c02 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -225,12 +225,11 @@ absFunc(sql_context * context, int argc, sql_value ** argv) return; } default:{ - /* Because sql_value_double() returns 0.0 if the argument is not - * something that can be converted into a number, we have: - * IMP: R-01992-00519 Abs(X) returns 0.0 if X is a string or blob + /* + * Abs(X) returns 0.0 if X is a string or blob * that cannot be converted to a numeric value. */ - double rVal = sql_value_double(argv[0]); + double rVal = mem_get_double_unsafe(argv[0]); if (rVal < 0) rVal = -rVal; sql_result_double(context, rVal); @@ -543,7 +542,7 @@ roundFunc(sql_context * context, int argc, sql_value ** argv) context->is_aborted = true; return; } - r = sql_value_double(argv[0]); + r = mem_get_double_unsafe(argv[0]); /* If Y==0 and X will fit in a 64-bit int, * handle the rounding directly, * otherwise use printf. @@ -1049,7 +1048,7 @@ quoteFunc(sql_context * context, int argc, sql_value ** argv) case MP_DOUBLE:{ double r1, r2; char zBuf[50]; - r1 = sql_value_double(argv[0]); + r1 = mem_get_double_unsafe(argv[0]); sql_snprintf(sizeof(zBuf), zBuf, "%!.15g", r1); sqlAtoF(zBuf, &r2, 20); if (r1 != r2) { @@ -1662,7 +1661,7 @@ sum_step(struct sql_context *context, int argc, sql_value **argv) p->overflow = 1; } } else { - p->rSum += sql_value_double(argv[0]); + p->rSum += mem_get_double_unsafe(argv[0]); p->approx = 1; } } diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c index ab31029df..764735322 100644 --- a/src/box/sql/mem.c +++ b/src/box/sql/mem.c @@ -1101,6 +1101,29 @@ mem_get_uint(const struct Mem *mem, uint64_t *u) return -1; } +int +mem_get_double(const struct Mem *mem, double *d) +{ + if ((mem->flags & MEM_Real) != 0) { + *d = mem->u.r; + return 0; + } + if ((mem->flags & MEM_Int) != 0) { + *d = (double)mem->u.i; + return 0; + } + if ((mem->flags & MEM_UInt) != 0) { + *d = (double)mem->u.u; + return 0; + } + if ((mem->flags & MEM_Str) != 0) { + if (sqlAtoF(mem->z, d, mem->n) == 0) + return -1; + return 0; + } + return -1; +} + int mem_copy(struct Mem *to, const struct Mem *from) { @@ -2240,32 +2263,6 @@ mem_value_bool(const struct Mem *mem, bool *b) return -1; } -/* - * Return the best representation of pMem that we can get into a - * double. If pMem is already a double or an integer, return its - * value. If it is a string or blob, try to convert it to a double. - * If it is a NULL, return 0.0. - */ -int -sqlVdbeRealValue(Mem * pMem, double *v) -{ - assert(EIGHT_BYTE_ALIGNMENT(pMem)); - if (pMem->flags & MEM_Real) { - *v = pMem->u.r; - return 0; - } else if (pMem->flags & MEM_Int) { - *v = (double)pMem->u.i; - return 0; - } else if ((pMem->flags & MEM_UInt) != 0) { - *v = (double)pMem->u.u; - return 0; - } else if (pMem->flags & MEM_Str) { - if (sqlAtoF(pMem->z, v, pMem->n)) - return 0; - } - return -1; -} - /**************************** sql_value_ ****************************** * The following routines extract information from a Mem or sql_value * structure. @@ -2292,14 +2289,6 @@ sql_value_bytes(sql_value * pVal) return sqlValueBytes(pVal); } -double -sql_value_double(sql_value * pVal) -{ - double v = 0.0; - sqlVdbeRealValue((Mem *) pVal, &v); - return v; -} - bool sql_value_boolean(sql_value *val) { diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h index ca8a75b50..d7ffa3444 100644 --- a/src/box/sql/mem.h +++ b/src/box/sql/mem.h @@ -788,6 +788,28 @@ mem_get_uint_unsafe(const struct Mem *mem) return u; } +/** + * Return value for MEM of DOUBLE type. For MEM of all other types convert + * value of the MEM to DOUBLE if possible and return converted value. Original + * MEM is not changed. + */ +int +mem_get_double(const struct Mem *mem, double *d); + +/** + * Return value of MEM converted to double. This function is not safe since + * there is no proper processing in case mem_get_double() return an error. In + * this case this functions returns 0. + */ +static inline double +mem_get_double_unsafe(const struct Mem *mem) +{ + double d; + if (mem_get_double(mem, &d) != 0) + return 0.; + return d; +} + /** * Simple type to str convertor. It is used to simplify * error reporting. @@ -842,16 +864,12 @@ releaseMemArray(Mem * p, int N); int mem_value_bool(const struct Mem *mem, bool *b); -int sqlVdbeRealValue(struct Mem *, double *); const void * sql_value_blob(struct Mem *); int sql_value_bytes(struct Mem *); -double -sql_value_double(struct Mem *); - bool sql_value_boolean(struct Mem *val); diff --git a/src/box/sql/printf.c b/src/box/sql/printf.c index eb8413f9c..2f1948ff8 100644 --- a/src/box/sql/printf.c +++ b/src/box/sql/printf.c @@ -152,7 +152,7 @@ getDoubleArg(PrintfArguments * p) { if (p->nArg <= p->nUsed) return 0.0; - return sql_value_double(p->apArg[p->nUsed++]); + return mem_get_double_unsafe(p->apArg[p->nUsed++]); } static char * diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index 6ead9b261..3f6fa1722 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -442,9 +442,6 @@ sql_column_bytes(sql_stmt *, int iCol); int sql_column_bytes16(sql_stmt *, int iCol); -double -sql_column_double(sql_stmt *, int iCol); - bool sql_column_boolean(struct sql_stmt *stmt, int column); diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index f7b6df0d9..1deb8e507 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -2751,24 +2751,22 @@ case OP_SeekGT: { /* jump, in3 */ if (mem_is_str(pIn3)) mem_to_number(pIn3); int64_t i; - if (mem_is_uint(pIn3)) { - i = pIn3->u.u; - is_neg = false; - } else if (mem_is_nint(pIn3)) { - i = pIn3->u.i; - is_neg = true; - } else if (mem_is_double(pIn3)) { - if (pIn3->u.r > (double)INT64_MAX) + if (mem_get_int(pIn3, &i, &is_neg) != 0) { + if (!mem_is_double(pIn3)) { + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, + mem_str(pIn3), "integer"); + goto abort_due_to_error; + } + double d = mem_get_double_unsafe(pIn3); + assert(d >= (double)INT64_MAX || d < (double)INT64_MIN); + /* TODO: add [INT64_MAX, UINT64_MAX) here. */ + if (d > (double)INT64_MAX) i = INT64_MAX; - else if (pIn3->u.r < (double)INT64_MIN) + else if (d < (double)INT64_MIN) i = INT64_MIN; else - i = pIn3->u.r; + i = d; is_neg = i < 0; - } else { - diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - mem_str(pIn3), "integer"); - goto abort_due_to_error; } iKey = i; @@ -2791,7 +2789,7 @@ case OP_SeekGT: { /* jump, in3 */ * (x > 4.9) -> (x >= 5) * (x <= 4.9) -> (x < 5) */ - if (pIn3->u.r<(double)iKey) { + if (mem_get_double_unsafe(pIn3) < (double)iKey) { assert(OP_SeekGE==(OP_SeekGT-1)); assert(OP_SeekLT==(OP_SeekLE-1)); assert((OP_SeekLE & 0x0001)==(OP_SeekGT & 0x0001)); @@ -2801,7 +2799,7 @@ case OP_SeekGT: { /* jump, in3 */ /* If the approximation iKey is smaller than the actual real search * term, substitute <= for < and > for >=. */ - else if (pIn3->u.r>(double)iKey) { + else if (mem_get_double_unsafe(pIn3) > (double)iKey) { assert(OP_SeekLE==(OP_SeekLT+1)); assert(OP_SeekGT==(OP_SeekGE+1)); assert((OP_SeekLT & 0x0001)==(OP_SeekGE & 0x0001)); diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c index 1126425bc..daa5e0809 100644 --- a/src/box/sql/vdbeapi.c +++ b/src/box/sql/vdbeapi.c @@ -464,12 +464,6 @@ sql_column_bytes(sql_stmt * pStmt, int i) return sql_value_bytes(columnMem(pStmt, i)); } -double -sql_column_double(sql_stmt * pStmt, int i) -{ - return sql_value_double(columnMem(pStmt, i)); -} - bool sql_column_boolean(struct sql_stmt *stmt, int i) {