[Tarantool-patches] [PATCH v5 47/52] sql: introduce mem_get_double()
Mergen Imeev
imeevma at tarantool.org
Wed Apr 14 04:00:15 MSK 2021
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 <imeevma at gmail.com>
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)
{
More information about the Tarantool-patches
mailing list