[Tarantool-patches] [PATCH 0/4] sql: fix NUMBER conversion issues

[Tarantool-patches] [PATCH 0/4] sql: fix NUMBER conversion issues

[Nikita Pettik]

Branch: https://github.com/tarantool/tarantool/tree/np/gh-4233-fix-number-field-type-in-sql
Issue:
https://github.com/tarantool/tarantool/issues/4233
https://github.com/tarantool/tarantool/issues/4463

This patch-set makes NUMBER behaviour be closer to the one defined
in Tarantool NoSQL.

This is rework of https://github.com/tarantool/tarantool/tree/imeevma/gh-4233-fix-number-field-type-in-sql
Functional changes: fixed CAST boolean AS NUMBER operation - now it is
allowed to cast booleans to number type (since boolean can be casted to
integer, and in turn NUMBER type comprises integer type).
Refactoring: splitted patch in a series of independent ones; fixed
codestyle of sqlVdbeMemNumerify().

To the one who will review: this patch-series goes-around since
athour of original patch does not agree with suggested (by me) fixes.
Hence I guess second review iteration is required.

Nikita Pettik (4):
  sql: remove cast to INT during FP arithmetic ops
  sql: refactor sqlVdbeMemNumerify()
  sql: fix CAST AS NUMBER operator
  sql: do not force FP representation for NUMBER field

 src/box/sql/vdbe.c                   |  15 +----
 src/box/sql/vdbeInt.h                |  12 +++-
 src/box/sql/vdbemem.c                |  65 +++++++--------------
 test/sql-tap/cast.test.lua           |  32 +++++------
 test/sql-tap/e_select1.test.lua      |   2 +-
 test/sql-tap/numcast.test.lua        | 106 ++++++++++++++++++++++++++++++++++-
 test/sql-tap/sort.test.lua           |  12 ++--
 test/sql-tap/tkt-91e2e8ba6f.test.lua |  12 ++--
 test/sql/boolean.result              |   9 ++-
 test/sql/integer-overflow.result     |   2 +-
 test/sql/types.result                |   2 +-
 11 files changed, 176 insertions(+), 93 deletions(-)

-- 
2.15.1

[Tarantool-patches] [PATCH 1/4] sql: remove cast to INT during FP arithmetic ops

[Nikita Pettik]

Arithmetic operations are implemented by OP_Add, OP_Substract etc VDBE
opcodes which consist of almost the same internal logic: depending on
type of operands (integer of FP) execution flow jumps to the one of two
branches.  At this point branch which is responsible for floating point
operations finishes with next code:

  1668			if (((type1|type2)&MEM_Real)==0 && !bIntint) {
  1669				mem_apply_integer_type(pOut);
  1670			}

At least one type of type1 and type2 is supposed to be MEM_Real.
Otherwise, execution flow either hits branch processing integer
arithmetic operations or VDBE execution is aborted with
ER_SQL_TYPE_MISMATCH Thus, condition under 'if' clause is always
evaluated to 'false' value ergo mem_apply_integer_type() is never
called. Let's remove this dead code.

Implemented by Mergen Imeev <imeevma@tarantool.org>
---
 src/box/sql/vdbe.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index eab74db4a..a5f161d90 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1566,7 +1566,6 @@ case OP_Subtract:              /* same as TK_MINUS, in1, in2, out3 */
 case OP_Multiply:              /* same as TK_STAR, in1, in2, out3 */
 case OP_Divide:                /* same as TK_SLASH, in1, in2, out3 */
 case OP_Remainder: {           /* same as TK_REM, in1, in2, out3 */
-	char bIntint;   /* Started out as two integer operands */
 	u32 flags;      /* Combined MEM_* flags from both inputs */
 	u16 type1;      /* Numeric type of left operand */
 	u16 type2;      /* Numeric type of right operand */
@@ -1589,7 +1588,6 @@ case OP_Remainder: {           /* same as TK_REM, in1, in2, out3 */
 		bool is_lhs_neg = pIn1->flags & MEM_Int;
 		bool is_rhs_neg = pIn2->flags & MEM_Int;
 		bool is_res_neg;
-		bIntint = 1;
 		switch( pOp->opcode) {
 		case OP_Add: {
 			if (sql_add_int(iA, is_lhs_neg, iB, is_rhs_neg,
@@ -1629,7 +1627,6 @@ case OP_Remainder: {           /* same as TK_REM, in1, in2, out3 */
 		}
 		mem_set_int(pOut, iB, is_res_neg);
 	} else {
-		bIntint = 0;
 		if (sqlVdbeRealValue(pIn1, &rA) != 0) {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 				 sql_value_to_diag_str(pIn1), "numeric");
@@ -1640,6 +1637,7 @@ case OP_Remainder: {           /* same as TK_REM, in1, in2, out3 */
 				 sql_value_to_diag_str(pIn2), "numeric");
 			goto abort_due_to_error;
 		}
+		assert(((type1 | type2) & MEM_Real) != 0);
 		switch( pOp->opcode) {
 		case OP_Add:         rB += rA;       break;
 		case OP_Subtract:    rB -= rA;       break;
@@ -1665,9 +1663,6 @@ case OP_Remainder: {           /* same as TK_REM, in1, in2, out3 */
 		}
 		pOut->u.r = rB;
 		MemSetTypeFlag(pOut, MEM_Real);
-		if (((type1|type2)&MEM_Real)==0 && !bIntint) {
-			mem_apply_integer_type(pOut);
-		}
 	}
 	break;
 
-- 
2.15.1

[Tarantool-patches] [PATCH 2/4] sql: refactor sqlVdbeMemNumerify()

[Nikita Pettik]

Fix codestyle and comment; allow conversion from boolean to number
(since it is legal to convert boolean to integer, and in turn number
type completely includes integer type).

Part of #4233
---
 src/box/sql/vdbeInt.h | 12 +++++++++++-
 src/box/sql/vdbemem.c | 46 +++++++++++++++++++---------------------------
 2 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index 1393f3fd2..a80a691e1 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -533,7 +533,17 @@ mem_value_bool(const struct Mem *mem, bool *b);
 
 int mem_apply_integer_type(Mem *);
 int sqlVdbeMemRealify(Mem *);
-int sqlVdbeMemNumerify(Mem *);
+
+/**
+ * Convert @mem to NUMBER type, so that after conversion it has one
+ * of types MEM_Real, MEM_Int or MEM_UInt. If conversion is not possible,
+ * function returns -1.
+ *
+ * Beware - this function changes value and type of @mem argument
+ */
+int
+vdbe_mem_numerify(struct Mem *mem);
+
 int sqlVdbeMemCast(Mem *, enum field_type type);
 int sqlVdbeMemFromBtree(BtCursor *, u32, u32, Mem *);
 void sqlVdbeMemRelease(Mem * p);
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index df3f0d8b1..a533a2fe7 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -596,34 +596,26 @@ sqlVdbeMemRealify(Mem * pMem)
 	return 0;
 }
 
-/*
- * Convert pMem so that it has types MEM_Real or MEM_Int or both.
- * Invalidate any prior representations.
- *
- * Every effort is made to force the conversion, even if the input
- * is a string that does not look completely like a number.  Convert
- * as much of the string as we can and ignore the rest.
- */
 int
-sqlVdbeMemNumerify(Mem * pMem)
+vdbe_mem_numerify(struct Mem *mem)
 {
-	if ((pMem->flags & (MEM_Int | MEM_UInt | MEM_Real | MEM_Null)) == 0) {
-		assert((pMem->flags & (MEM_Blob | MEM_Str)) != 0);
-		bool is_neg;
-		int64_t i;
-		if (sql_atoi64(pMem->z, &i, &is_neg, pMem->n) == 0) {
-			mem_set_int(pMem, i, is_neg);
-		} else {
-			double v;
-			if (sqlVdbeRealValue(pMem, &v))
-				return -1;
-			pMem->u.r = v;
-			MemSetTypeFlag(pMem, MEM_Real);
-			mem_apply_integer_type(pMem);
-		}
+	if ((mem->flags & (MEM_Int | MEM_UInt | MEM_Real | MEM_Null)) != 0)
+		return 0;
+	if ((mem->flags & MEM_Bool) != 0) {
+		mem->u.u = mem->u.b;
+		MemSetTypeFlag(mem, MEM_UInt);
+		return 0;
+	}
+	assert((mem->flags & (MEM_Blob | MEM_Str)) != 0);
+	bool is_neg;
+	int64_t i;
+	if (sql_atoi64(mem->z, &i, &is_neg, mem->n) == 0) {
+		mem_set_int(mem, i, is_neg);
+	} else {
+		if (sqlAtoF(mem->z, &mem->u.r, mem->n) == 0)
+			return -1;
+		MemSetTypeFlag(mem, MEM_Real);
 	}
-	assert((pMem->flags & (MEM_Int | MEM_UInt | MEM_Real | MEM_Null)) != 0);
-	pMem->flags &= ~(MEM_Str | MEM_Blob | MEM_Zero);
 	return 0;
 }
 
@@ -1472,7 +1464,7 @@ valueFromExpr(sql * db,	/* The database connection */
 		if (0 ==
 		    sqlValueFromExpr(db, pExpr->pLeft, type, &pVal)
 		    && pVal != 0) {
-			if ((rc = sqlVdbeMemNumerify(pVal)) != 0)
+			if ((rc = vdbe_mem_numerify(pVal)) != 0)
 				return rc;
 			if (pVal->flags & MEM_Real) {
 				pVal->u.r = -pVal->u.r;
@@ -1499,7 +1491,7 @@ valueFromExpr(sql * db,	/* The database connection */
 		pVal = valueNew(db, pCtx);
 		if (pVal == 0)
 			goto no_mem;
-		if ((rc = sqlVdbeMemNumerify(pVal)) != 0)
+		if ((rc = vdbe_mem_numerify(pVal)) != 0)
 			return rc;
 	}
 #ifndef SQL_OMIT_BLOB_LITERAL
-- 
2.15.1

[Tarantool-patches] [PATCH 3/4] sql: fix CAST AS NUMBER operator

[Nikita Pettik]

NUMBER type is supposed to include values of both integer and FP types.
Hence, if numeric value is casted to NUMBER it remains unchanged.
Before this patch cast to NUMBER always resulted in forcing floating
point representation. Furthermore, CAST of blob values to NUMBER always
led the floating point result, even if blob value had precise integer
representation. Since now NUMBER doesn't imply only FP values, let's fix
this and use vdbe_mem_numerify() which provides unified way of casting
to NUMBER type.

Part of #4233
Closes #4463
---
 src/box/sql/vdbemem.c           | 19 ++---------------
 test/sql-tap/cast.test.lua      | 32 ++++++++++++++--------------
 test/sql-tap/e_select1.test.lua |  2 +-
 test/sql-tap/numcast.test.lua   | 46 ++++++++++++++++++++++++++++++++++++++++-
 test/sql/boolean.result         |  9 ++++++--
 test/sql/types.result           |  2 +-
 6 files changed, 72 insertions(+), 38 deletions(-)

diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index a533a2fe7..aad030df9 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -668,22 +668,6 @@ sqlVdbeMemCast(Mem * pMem, enum field_type type)
 	assert(type < field_type_MAX);
 	if (pMem->flags & MEM_Null)
 		return 0;
-	if ((pMem->flags & MEM_Blob) != 0 && type == FIELD_TYPE_NUMBER) {
-		bool is_neg;
-		if (sql_atoi64(pMem->z,  (int64_t *) &pMem->u.i, &is_neg,
-			       pMem->n) == 0) {
-			MemSetTypeFlag(pMem, MEM_Real);
-			if (is_neg)
-				pMem->u.r = pMem->u.i;
-			else
-				pMem->u.r = pMem->u.u;
-			return 0;
-		}
-		if (sqlAtoF(pMem->z, &pMem->u.r, pMem->n) == 0)
-			return -1;
-		MemSetTypeFlag(pMem, MEM_Real);
-		return 0;
-	}
 	switch (type) {
 	case FIELD_TYPE_SCALAR:
 		return 0;
@@ -734,8 +718,9 @@ sqlVdbeMemCast(Mem * pMem, enum field_type type)
 			return -1;
 		return 0;
 	case FIELD_TYPE_DOUBLE:
-	case FIELD_TYPE_NUMBER:
 		return sqlVdbeMemRealify(pMem);
+	case FIELD_TYPE_NUMBER:
+		return vdbe_mem_numerify(pMem);
 	case FIELD_TYPE_VARBINARY:
 		if ((pMem->flags & MEM_Blob) != 0)
 			return 0;
diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua
index 9c937a065..a3f0bc787 100755
--- a/test/sql-tap/cast.test.lua
+++ b/test/sql-tap/cast.test.lua
@@ -628,14 +628,14 @@ test:do_execsql_test(
         SELECT CAST(9223372036854774800 AS NUMBER)
     ]], {
         -- <cast-3.2>
-        9223372036854774784
+        9223372036854774800LL
         -- </cast-3.2>
     })
 
 test:do_execsql_test(
     "cast-3.3",
     "SELECT CAST(9223372036854774800 AS NUMBER)",
-    {9.22337203685477e+18})
+    {9223372036854774800LL})
 
 test:do_execsql_test(
     "cast-3.4",
@@ -643,7 +643,7 @@ test:do_execsql_test(
         SELECT CAST(CAST(9223372036854774800 AS NUMBER) AS integer)
     ]], {
         -- <cast-3.4>
-        9223372036854774784LL
+        9223372036854774800LL
         -- </cast-3.4>
     })
 
@@ -663,14 +663,14 @@ test:do_execsql_test(
         SELECT CAST(-9223372036854774800 AS NUMBER)
     ]], {
         -- <cast-3.6>
-        -9223372036854774784
+        -9223372036854774800LL
         -- </cast-3.6>
     })
 
 test:do_execsql_test(
     "cast-3.7",
     "SELECT CAST(-9223372036854774800 AS NUMBER)",
-    {-9.22337203685477e+18})
+    {-9223372036854774800LL})
 
 test:do_execsql_test(
     "cast-3.8",
@@ -678,7 +678,7 @@ test:do_execsql_test(
         SELECT CAST(CAST(-9223372036854774800 AS NUMBER) AS integer)
     ]], {
         -- <cast-3.8>
-        -9223372036854774784LL
+        -9223372036854774800LL
         -- </cast-3.8>
     })
 
@@ -695,7 +695,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "cast-3.12",
     [[
-        SELECT CAST('9223372036854774800' AS NUMBER)
+        SELECT CAST('9223372036854774800.' AS NUMBER)
     ]], {
         -- <cast-3.12>
         9223372036854774784
@@ -705,12 +705,12 @@ test:do_execsql_test(
 test:do_execsql_test(
     "cast-3.13",
     "SELECT CAST(9223372036854774800 AS NUMBER)",
-    {9.22337203685477e+18})
+    {9223372036854774800LL})
 
 test:do_execsql_test(
     "cast-3.14",
     [[
-        SELECT CAST(CAST('9223372036854774800' AS NUMBER) AS integer)
+        SELECT CAST(CAST('9223372036854774800.' AS NUMBER) AS integer)
     ]], {
         -- <cast-3.14>
         9223372036854774784LL
@@ -732,7 +732,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "cast-3.16",
     [[
-        SELECT CAST('-9223372036854774800' AS NUMBER)
+        SELECT CAST('-9223372036854774800.' AS NUMBER)
     ]], {
         -- <cast-3.16>
         -9223372036854774784
@@ -741,13 +741,13 @@ test:do_execsql_test(
 
 test:do_execsql_test(
     "cast-3.17",
-    "SELECT CAST('-9223372036854774800' AS NUMBER)",
+    "SELECT CAST('-9223372036854774800.' AS NUMBER)",
     {-9.22337203685477e+18})
 
 test:do_execsql_test(
     "cast-3.18",
     [[
-        SELECT CAST(CAST('-9223372036854774800' AS NUMBER) AS integer)
+        SELECT CAST(CAST('-9223372036854774800.' AS NUMBER) AS integer)
     ]], {
         -- <cast-3.18>
         -9223372036854774784LL
@@ -770,7 +770,7 @@ if true then --test:execsql("PRAGMA encoding")[1][1]=="UTF-8" then
     test:do_execsql_test(
         "cast-3.22",
         [[
-            SELECT CAST(x'39323233333732303336383534373734383030' AS NUMBER)
+            SELECT CAST(x'393232333337323033363835343737343830302E' AS NUMBER)
         ]], {
             -- <cast-3.22>
             9223372036854774784
@@ -778,7 +778,7 @@ if true then --test:execsql("PRAGMA encoding")[1][1]=="UTF-8" then
         })
     test:do_execsql_test(
         "cast-3.23",
-        "SELECT CAST(x'39323233333732303336383534373734383030' AS NUMBER)",
+        "SELECT CAST(x'393232333337323033363835343737343830302E' AS NUMBER)",
         {9.22337203685477e+18})
 
     test:do_execsql_test(
@@ -788,7 +788,7 @@ if true then --test:execsql("PRAGMA encoding")[1][1]=="UTF-8" then
                         AS integer)
         ]], {
             -- <cast-3.24>
-            9223372036854774784LL
+        9223372036854774800LL
             -- </cast-3.24>
         })
 end
@@ -796,7 +796,7 @@ end
 test:do_execsql_test(
     "case-3.25",
     [[
-        SELECT CAST(x'3138343436373434303733373039353531363135' AS NUMBER);
+        SELECT CAST(x'31383434363734343037333730393535313631352E' AS NUMBER);
     ]], { 1.844674407371e+19 } )
 
 test:do_execsql_test(
diff --git a/test/sql-tap/e_select1.test.lua b/test/sql-tap/e_select1.test.lua
index 48d8c226e..c1818ddb1 100755
--- a/test/sql-tap/e_select1.test.lua
+++ b/test/sql-tap/e_select1.test.lua
@@ -760,7 +760,7 @@ test:do_execsql_test(
         INSERT INTO z1 VALUES(6, 63, '0', -26);
 
         INSERT INTO z2 VALUES(1, NULL, 21);
-        INSERT INTO z2 VALUES(2, 36, 6);
+        INSERT INTO z2 VALUES(2, 36.0, 6.0);
 
         INSERT INTO z3 VALUES(1, 123.21, 123.12);
         INSERT INTO z3 VALUES(2, 49.17, -67);
diff --git a/test/sql-tap/numcast.test.lua b/test/sql-tap/numcast.test.lua
index 07117d08e..87c5f6b35 100755
--- a/test/sql-tap/numcast.test.lua
+++ b/test/sql-tap/numcast.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(20)
+test:plan(25)
 
 --!./tcltestrunner.lua
 -- 2013 March 20
@@ -147,4 +147,48 @@ test:do_catchsql_test(
         1,"Tuple field 1 type does not match one required by operation: expected integer"
     })
 
+--
+-- gh-4233: Make sure that NUMBER can contain UNSIGNED, INTEGER
+-- and DOUBLE and is not automatically converted to DOUBLE.
+--
+test:do_execsql_test(
+    "numcast-3.1",
+    [[
+        SELECT CAST(x'3131313131313131313131313131313131313131' AS NUMBER);
+    ]], {
+        11111111111111111111ULL
+    })
+
+test:do_execsql_test(
+    "numcast-3.2",
+    [[
+        SELECT CAST(x'31313131313131313131313131313131313131312E' AS NUMBER);
+    ]], {
+        11111111111111110656
+    })
+
+test:do_execsql_test(
+    "numcast-3.3",
+    [[
+        SELECT CAST('11111111111111111111' AS NUMBER);
+    ]], {
+        11111111111111111111ULL
+    })
+
+test:do_execsql_test(
+    "numcast-3.4",
+    [[
+        SELECT CAST('101' AS NUMBER) / 10, CAST('101.' AS NUMBER) / 10;
+    ]], {
+        10, 10.1
+    })
+
+test:do_execsql_test(
+    "numcast-3.5",
+    [[
+        SELECT CAST('101     ' AS NUMBER) / 10, CAST('      101' AS NUMBER) / 10;
+    ]], {
+        10, 10
+    })
+
 test:finish_test()
diff --git a/test/sql/boolean.result b/test/sql/boolean.result
index 7769d0cb3..112e41a12 100644
--- a/test/sql/boolean.result
+++ b/test/sql/boolean.result
@@ -510,8 +510,13 @@ SELECT cast(true AS INTEGER), cast(false AS INTEGER);
  | ...
 SELECT cast(true AS NUMBER), cast(false AS NUMBER);
  | ---
- | - null
- | - 'Type mismatch: can not convert TRUE to number'
+ | - metadata:
+ |   - name: cast(true AS NUMBER)
+ |     type: number
+ |   - name: cast(false AS NUMBER)
+ |     type: number
+ |   rows:
+ |   - [1, 0]
  | ...
 -- gh-4462: ensure that text representation is uppercase.
 SELECT cast(true AS TEXT), cast(false AS TEXT);
diff --git a/test/sql/types.result b/test/sql/types.result
index f0c34b6fa..38e4385ad 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -1023,7 +1023,7 @@ box.execute("SELECT CAST(18446744073709551615 AS NUMBER);")
   - name: CAST(18446744073709551615 AS NUMBER)
     type: number
   rows:
-  - [1.844674407371e+19]
+  - [18446744073709551615]
 ...
 box.execute("SELECT CAST(18446744073709551615 AS TEXT);")
 ---
-- 
2.15.1

[Tarantool-patches] [PATCH 4/4] sql: do not force FP representation for NUMBER field

[Nikita Pettik]

During value decoding fetched from space's field FP representation was
forced in case type of field was NUMBER. It was so since NUMBER used to
substitute DOUBLE field type (in fact NUMBER mimicked DOUBLE type). Since
now DOUBLE is a separate field type, there's no such necessity. Hence from
now integers from NUMBER field are treated as integers.

Implemented by Mergen Imeev <imeevma@gmail.com>

Closes #4233

@TarantoolBot document
Title: NUMBER column type changes

From now NUMBER behaves in the same way as in NoSQL Tarantool.
Previously, NUMBER was rather synonym to what now DOUBLE means: it used
to force floating point representation of values, even if they were
integers. A few examples:

1) CAST operation:

Obsolete behaviour:
SELECT CAST(922337206854774800 AS NUMBER), CAST(5 AS NUMBER) / 10;
---
 rows:
- [922337206854774784, 0.5]

New behaviour:
SELECT CAST(922337206854774800 AS NUMBER), CAST(5 AS NUMBER) / 10;
---
 rows:
- [922337206854774800, 0]

2) Preserving integer representation:

Obsolete behaviour:
CREATE TABLE t (n NUMBER PRIMARY KEY);
INSERT INTO t VALUES (3), (-4), (5.0);
SELECT n, n/10 FROM t;
---
 rows:
- [-4, -0.4]
- [3, 0.3]
- [5, 0.5]

New behaviour:
SELECT n, n/10 FROM t;
---
 rows:
- [-4, 0]
- [3, 0]
- [5, 0.5]
---
 src/box/sql/vdbe.c                   |  8 -----
 test/sql-tap/numcast.test.lua        | 62 +++++++++++++++++++++++++++++++++++-
 test/sql-tap/sort.test.lua           | 12 +++----
 test/sql-tap/tkt-91e2e8ba6f.test.lua | 12 +++----
 test/sql/integer-overflow.result     |  2 +-
 5 files changed, 74 insertions(+), 22 deletions(-)

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index a5f161d90..620d74e66 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2721,14 +2721,6 @@ case OP_Column: {
 	    default_val_mem != NULL) {
 		sqlVdbeMemShallowCopy(pDest, default_val_mem, MEM_Static);
 	}
-	if ((pDest->flags & (MEM_Int | MEM_UInt)) != 0) {
-		if (field_type == FIELD_TYPE_NUMBER) {
-			if ((pDest->flags & MEM_Int) != 0)
-				sqlVdbeMemSetDouble(pDest, pDest->u.i);
-			else
-				sqlVdbeMemSetDouble(pDest, pDest->u.u);
-		}
-	}
 	pDest->field_type = field_type;
 op_column_out:
 	REGISTER_TRACE(p, pOp->p3, pDest);
diff --git a/test/sql-tap/numcast.test.lua b/test/sql-tap/numcast.test.lua
index 87c5f6b35..f795cef75 100755
--- a/test/sql-tap/numcast.test.lua
+++ b/test/sql-tap/numcast.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(25)
+test:plan(31)
 
 --!./tcltestrunner.lua
 -- 2013 March 20
@@ -191,4 +191,64 @@ test:do_execsql_test(
         10, 10
     })
 
+test:do_execsql_test(
+    "numcast-3.6",
+    [[
+        CREATE TABLE t1 (id INT PRIMARY KEY, n NUMBER);
+        INSERT INTO t1 VALUES (1, 9223372036854775807);
+        INSERT INTO t1 VALUES (2, -9223372036854775807);
+        INSERT INTO t1 VALUES (3, 9223372036854775807.1);
+        SELECT n, n/100 FROM t1;
+    ]], {
+        9223372036854775807ULL, 92233720368547758ULL,
+        -9223372036854775807LL, -92233720368547758LL,
+        9223372036854775808, 92233720368547758.08
+    })
+
+test:do_execsql_test(
+    "numcast-3.7",
+    [[
+        CREATE TABLE t2(a NUMBER primary key);
+        INSERT INTO t2 VALUES(-56);
+        INSERT INTO t2 VALUES(44.0);
+        INSERT INTO t2 VALUES(46);
+        INSERT INTO t2 VALUES(56.0);
+        SELECT (a + 25) / 50 FROM t2;
+    ]], {
+        0,1.38,1,1.62
+})
+
+
+test:do_execsql_test(
+    "numcast-3.8",
+    [[
+        SELECT (1 + 0) / 3, (1 + 0.) / 3, (1 + 0) / 3.;
+    ]], {
+        0, 0.33333333333333, 0.33333333333333
+})
+
+test:do_execsql_test(
+    "numcast-3.9",
+    [[
+        SELECT (1 - 0) / 3, (1 - 0.) / 3, (1 - 0) / 3.;
+    ]], {
+        0, 0.33333333333333, 0.33333333333333
+})
+
+test:do_execsql_test(
+    "numcast-3.10",
+    [[
+        SELECT (1 * 1) / 3, (1 * 1.) / 3, (1 * 1) / 3.;
+    ]], {
+        0, 0.33333333333333, 0.33333333333333
+})
+
+test:do_execsql_test(
+    "numcast-3.11",
+    [[
+        SELECT (1 / 1) / 3, (1 / 1.) / 3, (1 / 1) / 3.;
+    ]], {
+        0 , 0.33333333333333, 0.33333333333333
+})
+
 test:finish_test()
diff --git a/test/sql-tap/sort.test.lua b/test/sql-tap/sort.test.lua
index e15641415..36074d6ef 100755
--- a/test/sql-tap/sort.test.lua
+++ b/test/sql-tap/sort.test.lua
@@ -243,7 +243,7 @@ test:do_execsql_test(
         SELECT v FROM t1 ORDER BY v;
     ]], {
         -- <sort-2.1.1>
-        "x-123.0", "x-2.15", "x-2b", "x-3.141592653", "x-4221.0", "x0.0013442", "x1.6", "x11.0"
+        "x-123.0", "x-2.15", "x-2b", "x-3.141592653", "x-4221.0", "x0.0013442", "x1.6", "x11"
         -- </sort-2.1.1>
     })
 
@@ -253,7 +253,7 @@ test:do_execsql_test(
         SELECT v FROM t1 ORDER BY substr(v,2,999);
     ]], {
         -- <sort-2.1.2>
-        "x-123.0", "x-2.15", "x-2b", "x-3.141592653", "x-4221.0", "x0.0013442", "x1.6", "x11.0"
+        "x-123.0", "x-2.15", "x-2b", "x-3.141592653", "x-4221.0", "x0.0013442", "x1.6", "x11"
         -- </sort-2.1.2>
     })
 
@@ -263,7 +263,7 @@ test:do_execsql_test(
         SELECT v FROM t1 ORDER BY substr(v,2,999) DESC;
     ]], {
         -- <sort-2.1.4>
-        "x11.0", "x1.6", "x0.0013442", "x-4221.0", "x-3.141592653", "x-2b", "x-2.15", "x-123.0"
+        "x11", "x1.6", "x0.0013442", "x-4221.0", "x-3.141592653", "x-2b", "x-2.15", "x-123.0"
         -- </sort-2.1.4>
     })
 
@@ -381,7 +381,7 @@ test:do_execsql_test(
         SELECT v FROM t1 ORDER BY 1;
     ]], {
         -- <sort-4.6>
-        "x-123.0", "x-2.15", "x-2b", "x-3.141592653", "x-4.0e9", "x-4221.0", "x0.0013442", "x01234567890123456789", "x1.6", "x11.0", "x2.7", "x5.0e10"
+        "x-123.0", "x-2.15", "x-2b", "x-3.141592653", "x-4.0e9", "x-4221.0", "x0.0013442", "x01234567890123456789", "x1.6", "x11", "x2.7", "x5.0e10"
         -- </sort-4.6>
     })
 
@@ -391,7 +391,7 @@ test:do_execsql_test(
         SELECT v FROM t1 ORDER BY 1 DESC;
     ]], {
         -- <sort-4.7>
-        "x5.0e10", "x2.7", "x11.0", "x1.6", "x01234567890123456789", "x0.0013442", "x-4221.0", "x-4.0e9", "x-3.141592653", "x-2b", "x-2.15", "x-123.0"
+        "x5.0e10", "x2.7", "x11", "x1.6", "x01234567890123456789", "x0.0013442", "x-4221.0", "x-4.0e9", "x-3.141592653", "x-2b", "x-2.15", "x-123.0"
         -- </sort-4.7>
     })
 
@@ -401,7 +401,7 @@ test:do_execsql_test(
         SELECT substr(v,2,99) FROM t1 ORDER BY 1;
     ]], {
         -- <sort-4.8>
-    "-123.0","-2.15","-2b","-3.141592653","-4.0e9","-4221.0","0.0013442","01234567890123456789","1.6","11.0","2.7","5.0e10"
+    "-123.0","-2.15","-2b","-3.141592653","-4.0e9","-4221.0","0.0013442","01234567890123456789","1.6","11","2.7","5.0e10"
         -- </sort-4.8>
     })
 
diff --git a/test/sql-tap/tkt-91e2e8ba6f.test.lua b/test/sql-tap/tkt-91e2e8ba6f.test.lua
index 7622f7536..b12b6e0f3 100755
--- a/test/sql-tap/tkt-91e2e8ba6f.test.lua
+++ b/test/sql-tap/tkt-91e2e8ba6f.test.lua
@@ -35,7 +35,7 @@ test:do_execsql_test(
         SELECT x/10, y/10 FROM t1;
     ]], {
         -- <1.2>
-        1, 1.1
+        1, 1
         -- </1.2>
     })
 
@@ -45,7 +45,7 @@ test:do_execsql_test(
         SELECT x/10, y/10 FROM (SELECT * FROM t1);
     ]], {
         -- <1.3>
-        1, 1.1
+        1, 1
         -- </1.3>
     })
 
@@ -55,7 +55,7 @@ test:do_execsql_test(
         SELECT x/10, y/10 FROM (SELECT * FROM t1 LIMIT 5 OFFSET 0);
     ]], {
         -- <1.4>
-        1, 1.1
+        1, 1
         -- </1.4>
     })
 
@@ -65,7 +65,7 @@ test:do_execsql_test(
         SELECT x/10, y/10 FROM (SELECT * FROM t1 LIMIT 5 OFFSET 0) LIMIT 5 OFFSET 0;
     ]], {
         -- <1.5>
-        1, 1.1
+        1, 1
         -- </1.5>
     })
 
@@ -77,7 +77,7 @@ test:do_execsql_test(
         LIMIT 5 OFFSET 0;
     ]], {
         -- <1.6>
-        1, 1.1
+        1, 1
         -- </1.6>
     })
 
@@ -92,7 +92,7 @@ test:do_execsql_test(
         SELECT a.x/10, a.y/10 FROM v1 AS a, t1 AS b WHERE a.x = b.x LIMIT 5 OFFSET 0;
     ]], {
         -- <1.7>
-        1, 1.1
+        1, 1
         -- </1.7>
     })
 
diff --git a/test/sql/integer-overflow.result b/test/sql/integer-overflow.result
index db5c2f725..6269cb547 100644
--- a/test/sql/integer-overflow.result
+++ b/test/sql/integer-overflow.result
@@ -170,7 +170,7 @@ box.execute("SELECT * FROM t;")
   - name: A
     type: number
   rows:
-  - [1, 1.844674407371e+19]
+  - [1, 18446744073709551615]
   - [2, -1]
 ...
 box.space.T:drop()
-- 
2.15.1

Re: [Tarantool-patches] [PATCH 0/4] sql: fix NUMBER conversion issues

[Vladislav Shpilevoy]

Hi! I remember about this patch, will review soon.

Re: [Tarantool-patches] [PATCH 4/4] sql: do not force FP representation for NUMBER field

[Vladislav Shpilevoy]

Thanks for the patch, Mergen too!

> diff --git a/test/sql-tap/numcast.test.lua b/test/sql-tap/numcast.test.lua
> index 87c5f6b35..f795cef75 100755
> --- a/test/sql-tap/numcast.test.lua
> +++ b/test/sql-tap/numcast.test.lua
> @@ -191,4 +191,64 @@ test:do_execsql_test(
>          10, 10
>      })
>  
> +test:do_execsql_test(
> +    "numcast-3.6",
> +    [[
> +        CREATE TABLE t1 (id INT PRIMARY KEY, n NUMBER);
> +        INSERT INTO t1 VALUES (1, 9223372036854775807);
> +        INSERT INTO t1 VALUES (2, -9223372036854775807);
> +        INSERT INTO t1 VALUES (3, 9223372036854775807.1);
> +        SELECT n, n/100 FROM t1;
> +    ]], {
> +        9223372036854775807ULL, 92233720368547758ULL,
> +        -9223372036854775807LL, -92233720368547758LL,
> +        9223372036854775808, 92233720368547758.08

Could you please select smaller values for the test, in
case the big values don't matter here? Because
9223372036854775807.1 turned into 9223372036854775808 looks
confusing.

Re: [Tarantool-patches] [PATCH 3/4] sql: fix CAST AS NUMBER operator

[Vladislav Shpilevoy]

Thanks for the patch!

See 5 comments below.

> diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua
> index 9c937a065..a3f0bc787 100755
> --- a/test/sql-tap/cast.test.lua
> +++ b/test/sql-tap/cast.test.lua
> @@ -663,14 +663,14 @@ test:do_execsql_test(
>          SELECT CAST(-9223372036854774800 AS NUMBER)
>      ]], {
>          -- <cast-3.6>
> -        -9223372036854774784
> +        -9223372036854774800LL
>          -- </cast-3.6>
>      })
>  
>  test:do_execsql_test(
>      "cast-3.7",
>      "SELECT CAST(-9223372036854774800 AS NUMBER)",
> -    {-9.22337203685477e+18})
> +    {-9223372036854774800LL})

1. 3.6 and 3.7 are the same. I propose to drop one.

>  test:do_execsql_test(
>      "cast-3.8",
> @@ -705,12 +705,12 @@ test:do_execsql_test(
>  test:do_execsql_test(
>      "cast-3.13",
>      "SELECT CAST(9223372036854774800 AS NUMBER)",
> -    {9.22337203685477e+18})
> +    {9223372036854774800LL})

2. 3.2, 3.3 and 3.13 are exactly the same. I propose to keep
only one.

> @@ -732,7 +732,7 @@ test:do_execsql_test(
>  test:do_execsql_test(
>      "cast-3.16",
>      [[
> -        SELECT CAST('-9223372036854774800' AS NUMBER)
> +        SELECT CAST('-9223372036854774800.' AS NUMBER)
>      ]], {
>          -- <cast-3.16>
>          -9223372036854774784

3. 3.16 and 3.17 are the same. Lets drop one.

> @@ -778,7 +778,7 @@ if true then --test:execsql("PRAGMA encoding")[1][1]=="UTF-8" then
>          })
>      test:do_execsql_test(
>          "cast-3.23",
> -        "SELECT CAST(x'39323233333732303336383534373734383030' AS NUMBER)",
> +        "SELECT CAST(x'393232333337323033363835343737343830302E' AS NUMBER)",
>          {9.22337203685477e+18})

4. 3.22 and 3.23 are the same. Drop one, please.

>      test:do_execsql_test(
> @@ -788,7 +788,7 @@ if true then --test:execsql("PRAGMA encoding")[1][1]=="UTF-8" then
>                          AS integer)
>          ]], {
>              -- <cast-3.24>
> -            9223372036854774784LL
> +        9223372036854774800LL

5. Indentation is a bit off.

>              -- </cast-3.24>
>          })
>  end

Re: [Tarantool-patches] [PATCH 2/4] sql: refactor sqlVdbeMemNumerify()

[Vladislav Shpilevoy]

Hi! Thanks for the patchset!

On 05/02/2020 17:19, Nikita Pettik wrote:
> Fix codestyle and comment; allow conversion from boolean to number
> (since it is legal to convert boolean to integer, and in turn number
> type completely includes integer type).
> 
> Part of #4233
> ---
>  src/box/sql/vdbeInt.h | 12 +++++++++++-
>  src/box/sql/vdbemem.c | 46 +++++++++++++++++++---------------------------
>  2 files changed, 30 insertions(+), 28 deletions(-)
> 
> diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
> index 1393f3fd2..a80a691e1 100644
> --- a/src/box/sql/vdbeInt.h
> +++ b/src/box/sql/vdbeInt.h
> @@ -533,7 +533,17 @@ mem_value_bool(const struct Mem *mem, bool *b);
>  
>  int mem_apply_integer_type(Mem *);
>  int sqlVdbeMemRealify(Mem *);
> -int sqlVdbeMemNumerify(Mem *);
> +
> +/**
> + * Convert @mem to NUMBER type, so that after conversion it has one

'@' is a command prefix in Doxygen. The word after '@' is considered
a command. I suppose you didn't mean command 'mem', but rather wanted
to mention a parameter called 'mem'. In that case I suggest '@a mem'.
We use '@a' usually, except some old files, which still can use '\a'
or something like this.

Re: [Tarantool-patches] [PATCH 2/4] sql: refactor sqlVdbeMemNumerify()

[Nikita Pettik]

On 11 Feb 00:25, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patchset!
> 
> On 05/02/2020 17:19, Nikita Pettik wrote:
> > Fix codestyle and comment; allow conversion from boolean to number
> > (since it is legal to convert boolean to integer, and in turn number
> > type completely includes integer type).
> > 
> > Part of #4233
> > ---
> >  src/box/sql/vdbeInt.h | 12 +++++++++++-
> >  src/box/sql/vdbemem.c | 46 +++++++++++++++++++---------------------------
> >  2 files changed, 30 insertions(+), 28 deletions(-)
> > 
> > diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
> > index 1393f3fd2..a80a691e1 100644
> > --- a/src/box/sql/vdbeInt.h
> > +++ b/src/box/sql/vdbeInt.h
> > @@ -533,7 +533,17 @@ mem_value_bool(const struct Mem *mem, bool *b);
> >  
> >  int mem_apply_integer_type(Mem *);
> >  int sqlVdbeMemRealify(Mem *);
> > -int sqlVdbeMemNumerify(Mem *);
> > +
> > +/**
> > + * Convert @mem to NUMBER type, so that after conversion it has one
> 
> '@' is a command prefix in Doxygen. The word after '@' is considered
> a command. I suppose you didn't mean command 'mem', but rather wanted
> to mention a parameter called 'mem'. In that case I suggest '@a mem'.
> We use '@a' usually, except some old files, which still can use '\a'
> or something like this.

Ok (still don't understand origins of @a as a parameter marker):

diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index a80a691e1..9ecf7a486 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -535,7 +535,7 @@ int mem_apply_integer_type(Mem *);
 int sqlVdbeMemRealify(Mem *);
 
 /**
- * Convert @mem to NUMBER type, so that after conversion it has one
+ * Convert @a mem to NUMBER type, so that after conversion it has one
  * of types MEM_Real, MEM_Int or MEM_UInt. If conversion is not possible,
  * function returns -1.
  *

I've also fixed commit message a bit:

sql: rework sqlVdbeMemNumerify()                                                   
                                                                                   
Fix codestyle and comment; allow conversion from boolean to number                 
(since it is legal to convert boolean to integer, and in turn number               
type completely includes integer type). Note that currently                        
sqlVdbeMemNumerify() is never called, so changes applied to it can't be            
tested. It is going to be used in the further patches.                             
                                                                                   
Part of #4233 

Re: [Tarantool-patches] [PATCH 3/4] sql: fix CAST AS NUMBER operator

[Nikita Pettik]

On 11 Feb 00:24, Vladislav Shpilevoy wrote:
> Thanks for the patch!
> 
> See 5 comments below.
> 
> > diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua
> > index 9c937a065..a3f0bc787 100755
> > --- a/test/sql-tap/cast.test.lua
> > +++ b/test/sql-tap/cast.test.lua
> > @@ -663,14 +663,14 @@ test:do_execsql_test(
> >          SELECT CAST(-9223372036854774800 AS NUMBER)
> >      ]], {
> >          -- <cast-3.6>
> > -        -9223372036854774784
> > +        -9223372036854774800LL
> >          -- </cast-3.6>
> >      })
> >  
> >  test:do_execsql_test(
> >      "cast-3.7",
> >      "SELECT CAST(-9223372036854774800 AS NUMBER)",
> > -    {-9.22337203685477e+18})
> > +    {-9223372036854774800LL})
> 
> 1. 3.6 and 3.7 are the same. I propose to drop one.
> 
> >  test:do_execsql_test(
> >      "cast-3.8",
> > @@ -705,12 +705,12 @@ test:do_execsql_test(
> >  test:do_execsql_test(
> >      "cast-3.13",
> >      "SELECT CAST(9223372036854774800 AS NUMBER)",
> > -    {9.22337203685477e+18})
> > +    {9223372036854774800LL})
> 
> 2. 3.2, 3.3 and 3.13 are exactly the same. I propose to keep
> only one.
> 
> > @@ -732,7 +732,7 @@ test:do_execsql_test(
> >  test:do_execsql_test(
> >      "cast-3.16",
> >      [[
> > -        SELECT CAST('-9223372036854774800' AS NUMBER)
> > +        SELECT CAST('-9223372036854774800.' AS NUMBER)
> >      ]], {
> >          -- <cast-3.16>
> >          -9223372036854774784
> 
> 3. 3.16 and 3.17 are the same. Lets drop one.

Oh, thanks. Removed all duplicates from tests.
 
> > @@ -778,7 +778,7 @@ if true then --test:execsql("PRAGMA encoding")[1][1]=="UTF-8" then
> >          })
> >      test:do_execsql_test(
> >          "cast-3.23",
> > -        "SELECT CAST(x'39323233333732303336383534373734383030' AS NUMBER)",
> > +        "SELECT CAST(x'393232333337323033363835343737343830302E' AS NUMBER)",
> >          {9.22337203685477e+18})
> 
> 4. 3.22 and 3.23 are the same. Drop one, please.
> 
> >      test:do_execsql_test(
> > @@ -788,7 +788,7 @@ if true then --test:execsql("PRAGMA encoding")[1][1]=="UTF-8" then
> >                          AS integer)
> >          ]], {
> >              -- <cast-3.24>
> > -            9223372036854774784LL
> > +        9223372036854774800LL
> 
> 5. Indentation is a bit off.
> 
> >              -- </cast-3.24>
> >          })
> >  end

Diff:

diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua
index a3f0bc787..d4af339fd 100755
--- a/test/sql-tap/cast.test.lua
+++ b/test/sql-tap/cast.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(85)
+test:plan(80)
 
 --!./tcltestrunner.lua
 -- 2005 June 25
@@ -632,11 +632,6 @@ test:do_execsql_test(
         -- </cast-3.2>
     })
 
-test:do_execsql_test(
-    "cast-3.3",
-    "SELECT CAST(9223372036854774800 AS NUMBER)",
-    {9223372036854774800LL})
-
 test:do_execsql_test(
     "cast-3.4",
     [[
@@ -647,16 +642,6 @@ test:do_execsql_test(
         -- </cast-3.4>
     })
 
-test:do_execsql_test(
-    "cast-3.5",
-    [[
-        SELECT CAST(-9223372036854774800 AS integer)
-    ]], {
-        -- <cast-3.5>
-        -9223372036854774800LL
-        -- </cast-3.5>
-    })
-
 test:do_execsql_test(
     "cast-3.6",
     [[
@@ -667,11 +652,6 @@ test:do_execsql_test(
         -- </cast-3.6>
     })
 
-test:do_execsql_test(
-    "cast-3.7",
-    "SELECT CAST(-9223372036854774800 AS NUMBER)",
-    {-9223372036854774800LL})
-
 test:do_execsql_test(
     "cast-3.8",
     [[
@@ -739,11 +719,6 @@ test:do_execsql_test(
         -- </cast-3.16>
     })
 
-test:do_execsql_test(
-    "cast-3.17",
-    "SELECT CAST('-9223372036854774800.' AS NUMBER)",
-    {-9.22337203685477e+18})
-
 test:do_execsql_test(
     "cast-3.18",
     [[
@@ -776,10 +751,6 @@ if true then --test:execsql("PRAGMA encoding")[1][1]=="UTF-8" then
             9223372036854774784
             -- </cast-3.22>
         })
-    test:do_execsql_test(
-        "cast-3.23",
-        "SELECT CAST(x'393232333337323033363835343737343830302E' AS NUMBER)",
-        {9.22337203685477e+18})
 
     test:do_execsql_test(
         "cast-3.24",
@@ -788,7 +759,7 @@ if true then --test:execsql("PRAGMA encoding")[1][1]=="UTF-8" then
                         AS integer)
         ]], {
             -- <cast-3.24>
-        9223372036854774800LL
+            9223372036854774800LL
             -- </cast-3.24>
         })
 end

Re: [Tarantool-patches] [PATCH 4/4] sql: do not force FP representation for NUMBER field

[Nikita Pettik]

On 11 Feb 00:24, Vladislav Shpilevoy wrote:
> Thanks for the patch, Mergen too!
> 
> > diff --git a/test/sql-tap/numcast.test.lua b/test/sql-tap/numcast.test.lua
> > index 87c5f6b35..f795cef75 100755
> > --- a/test/sql-tap/numcast.test.lua
> > +++ b/test/sql-tap/numcast.test.lua
> > @@ -191,4 +191,64 @@ test:do_execsql_test(
> >          10, 10
> >      })
> >  
> > +test:do_execsql_test(
> > +    "numcast-3.6",
> > +    [[
> > +        CREATE TABLE t1 (id INT PRIMARY KEY, n NUMBER);
> > +        INSERT INTO t1 VALUES (1, 9223372036854775807);
> > +        INSERT INTO t1 VALUES (2, -9223372036854775807);
> > +        INSERT INTO t1 VALUES (3, 9223372036854775807.1);
> > +        SELECT n, n/100 FROM t1;
> > +    ]], {
> > +        9223372036854775807ULL, 92233720368547758ULL,
> > +        -9223372036854775807LL, -92233720368547758LL,
> > +        9223372036854775808, 92233720368547758.08
> 
> Could you please select smaller values for the test, in
> case the big values don't matter here? Because
> 9223372036854775807.1 turned into 9223372036854775808 looks
> confusing.

Ok:

diff --git a/test/sql-tap/numcast.test.lua b/test/sql-tap/numcast.test.lua
index f795cef75..eeac5353a 100755
--- a/test/sql-tap/numcast.test.lua
+++ b/test/sql-tap/numcast.test.lua
@@ -197,12 +197,12 @@ test:do_execsql_test(
         CREATE TABLE t1 (id INT PRIMARY KEY, n NUMBER);
         INSERT INTO t1 VALUES (1, 9223372036854775807);
         INSERT INTO t1 VALUES (2, -9223372036854775807);
-        INSERT INTO t1 VALUES (3, 9223372036854775807.1);
+        INSERT INTO t1 VALUES (3, 9007199254740992.0);
         SELECT n, n/100 FROM t1;
     ]], {
         9223372036854775807ULL, 92233720368547758ULL,
         -9223372036854775807LL, -92233720368547758LL,
-        9223372036854775808, 92233720368547758.08
+        9007199254740992, 90071992547409.92
     })

Re: [Tarantool-patches] [PATCH 2/4] sql: refactor sqlVdbeMemNumerify()

[Vladislav Shpilevoy]

Hi! Thanks for the fixes!

I've pushed my review fixes on top of this commit. See it in the
end of the email, and on the branch. I aligned the comment by
66 symbols, and changed '@mem' to '@a mem' in a second place.

Talking of your questions - why do we use @a? I think just because
it was needed to choose something and @a was chosen. From what I
remember, doxygen does not have a special command for mentioning
function parameters inside function's description. So there was no
a standard way.

================================================================================

commit bde8e58b05c5e4d3c650eea10e5905ce228b943d
Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Date:   Tue Feb 11 23:07:36 2020 +0100

    Review fixes

diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index 9ecf7a486..38305ce1d 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -535,11 +535,12 @@ int mem_apply_integer_type(Mem *);
 int sqlVdbeMemRealify(Mem *);
 
 /**
- * Convert @a mem to NUMBER type, so that after conversion it has one
- * of types MEM_Real, MEM_Int or MEM_UInt. If conversion is not possible,
- * function returns -1.
+ * Convert @a mem to NUMBER type, so that after conversion it has
+ * one of types MEM_Real, MEM_Int or MEM_UInt. If conversion is
+ * not possible, function returns -1.
  *
- * Beware - this function changes value and type of @mem argument
+ * Beware - this function changes value and type of @a mem
+ * argument.
  */
 int
 vdbe_mem_numerify(struct Mem *mem);

Re: [Tarantool-patches] [PATCH 3/4] sql: fix CAST AS NUMBER operator

[Vladislav Shpilevoy]

Hi!

I've pushed my review fixes on top of this commit. See it in the
end of the email, and on the branch.

3.2 and 3.13 still were the same, I dropped 3.13.

================================================================================

commit 6f1ce3265cab561c7d4959bd1a1998a986287da7
Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Date:   Tue Feb 11 23:14:48 2020 +0100

    Review fix

diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua
index d4af339fd..fb0790d04 100755
--- a/test/sql-tap/cast.test.lua
+++ b/test/sql-tap/cast.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(80)
+test:plan(79)
 
 --!./tcltestrunner.lua
 -- 2005 June 25
@@ -682,11 +682,6 @@ test:do_execsql_test(
         -- </cast-3.12>
     })
 
-test:do_execsql_test(
-    "cast-3.13",
-    "SELECT CAST(9223372036854774800 AS NUMBER)",
-    {9223372036854774800LL})
-
 test:do_execsql_test(
     "cast-3.14",
     [[

Re: [Tarantool-patches] [PATCH 3/4] sql: fix CAST AS NUMBER operator

[Nikita Pettik]

On 11 Feb 23:17, Vladislav Shpilevoy wrote:
> Hi!
> 
> I've pushed my review fixes on top of this commit. See it in the
> end of the email, and on the branch.
> 
> 3.2 and 3.13 still were the same, I dropped 3.13.

Don't bother with such insignificant things, I am always ok with these fixes.
Just amend them and force push already squashed :)

Ofc I've applied your fixes, thanks.

> ================================================================================
> 
> commit 6f1ce3265cab561c7d4959bd1a1998a986287da7
> Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
> Date:   Tue Feb 11 23:14:48 2020 +0100
> 
>     Review fix
> 
> diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua
> index d4af339fd..fb0790d04 100755
> --- a/test/sql-tap/cast.test.lua
> +++ b/test/sql-tap/cast.test.lua
> @@ -1,6 +1,6 @@
>  #!/usr/bin/env tarantool
>  test = require("sqltester")
> -test:plan(80)
> +test:plan(79)
>  
>  --!./tcltestrunner.lua
>  -- 2005 June 25
> @@ -682,11 +682,6 @@ test:do_execsql_test(
>          -- </cast-3.12>
>      })
>  
> -test:do_execsql_test(
> -    "cast-3.13",
> -    "SELECT CAST(9223372036854774800 AS NUMBER)",
> -    {9223372036854774800LL})
> -
>  test:do_execsql_test(
>      "cast-3.14",
>      [[

Re: [Tarantool-patches] [PATCH 3/4] sql: fix CAST AS NUMBER operator

[Vladislav Shpilevoy]

On 12/02/2020 00:20, Nikita Pettik wrote:
> On 11 Feb 23:17, Vladislav Shpilevoy wrote:
>> Hi!
>>
>> I've pushed my review fixes on top of this commit. See it in the
>> end of the email, and on the branch.
>>
>> 3.2 and 3.13 still were the same, I dropped 3.13.
> 
> Don't bother with such insignificant things, I am always ok with these fixes.
> Just amend them and force push already squashed :)

Ok, will take that into account next time

> Ofc I've applied your fixes, thanks.

The patchset LGTM then.

Re: [Tarantool-patches] [PATCH 3/4] sql: fix CAST AS NUMBER operator

[Nikita Pettik]

On 12 Feb 00:27, Vladislav Shpilevoy wrote:
> On 12/02/2020 00:20, Nikita Pettik wrote:
> > On 11 Feb 23:17, Vladislav Shpilevoy wrote:
> >> Hi!
> >>
> >> I've pushed my review fixes on top of this commit. See it in the
> >> end of the email, and on the branch.
> >>
> >> 3.2 and 3.13 still were the same, I dropped 3.13.
> > 
> > Don't bother with such insignificant things, I am always ok with these fixes.
> > Just amend them and force push already squashed :)
> 
> Ok, will take that into account next time
> 
> > Ofc I've applied your fixes, thanks.
> 
> The patchset LGTM then.

Pushed to master only. Milestone for this issue is 2.2.3, but 2.2 lacks
DOUBLE type, so patches can't be applied.

I did not push to 2.3 since I don't consider this issue to be a bug,
but rather behaviour change in type system. However, if somebody
insists on adding this change to 2.3 then I won't mind.