[tarantool-patches] [PATCH v2 10/15] sql: support -2^63 .. 2^64-1 integer type

[Stanislav Zudin <szudin@tarantool.org>]

Fixes an error in the conversion functions.
The cast to integer didn't take into account
the 'unsigned' bit.
Fixes the error in overflow check inside the sqlMulInt64().
Makes the overflow check more precisely in sql_atoi64().
Fixes the error message, and affected tests.

Part of #3810
---
 src/box/sql/util.c               |  5 +-
 src/box/sql/vdbe.c               | 24 ++++++---
 src/box/sql/vdbeInt.h            |  2 +-
 src/box/sql/vdbeapi.c            |  6 ++-
 src/box/sql/vdbemem.c            | 86 ++++++++++++++++++++------------
 test/sql/integer-overflow.result |  4 +-
 6 files changed, 80 insertions(+), 47 deletions(-)

diff --git a/src/box/sql/util.c b/src/box/sql/util.c
index d585dc0d5..1f3f92a04 100644
--- a/src/box/sql/util.c
+++ b/src/box/sql/util.c
@@ -614,10 +614,11 @@ sql_atoi64(const char *z, int64_t *val, int length)
 	}
 
 	char* end = NULL;
+	errno = 0;
 	u64 u = strtoull(z, &end, 10);
 	if (end < expected_end)
 		return ATOI_OVERFLOW;
-	if (errno == ERANGE)
+	if (errno != 0)
 		return ATOI_OVERFLOW;
 
 	enum atoi_result rc;
@@ -1404,7 +1405,7 @@ sqlMulInt64(i64 * pA, bool is_signedA, i64 iB, bool is_signedB)
 		if (INT64_MIN_MOD / uA < uB)
 			return ATHR_OVERFLOW;
 	} else {
-		if (INT64_MAX / uA < uB)
+		if (UINT64_MAX / uA < uB)
 			return ATHR_OVERFLOW;
 	}
 
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index ad2ce1787..997d0a1ab 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1755,7 +1755,7 @@ division_by_zero:
 	rc = SQL_TARANTOOL_ERROR;
 	goto abort_due_to_error;
 integer_overflow:
-	diag_set(ClientError, ER_SQL_EXECUTE, "integer is overflowed");
+	diag_set(ClientError, ER_SQL_EXECUTE, "integer overflow");
 	rc = SQL_TARANTOOL_ERROR;
 	goto abort_due_to_error;
 }
@@ -1941,13 +1941,15 @@ case OP_ShiftRight: {           /* same as TK_RSHIFT, in1, in2, out3 */
 		sqlVdbeMemSetNull(pOut);
 		break;
 	}
-	if (sqlVdbeIntValue(pIn2, (int64_t *) &iA) != 0) {
+	bool is_unsignedA = false;
+	bool is_unsignedB = false;
+	if (sqlVdbeIntValue(pIn2, (int64_t *) &iA, &is_unsignedA) != 0) {
 		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 			 sql_value_text(pIn2), "integer");
 		rc = SQL_TARANTOOL_ERROR;
 		goto abort_due_to_error;
 	}
-	if (sqlVdbeIntValue(pIn1, (int64_t *) &iB) != 0) {
+	if (sqlVdbeIntValue(pIn1, (int64_t *) &iB, &is_unsignedB) != 0) {
 		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 			 sql_value_text(pIn1), "integer");
 		rc = SQL_TARANTOOL_ERROR;
@@ -1961,6 +1963,10 @@ case OP_ShiftRight: {           /* same as TK_RSHIFT, in1, in2, out3 */
 	} else if (iB!=0) {
 		assert(op==OP_ShiftRight || op==OP_ShiftLeft);
 
+		if (is_unsignedB){
+			/* Limit big unsigned values by 64 */
+			iB = 64;
+		}
 		/* If shifting by a negative amount, shift in the other direction */
 		if (iB<0) {
 			assert(OP_ShiftRight==OP_ShiftLeft+1);
@@ -2468,7 +2474,8 @@ case OP_Or: {             /* same as TK_OR, in1, in2, out3 */
 		v1 = 2;
 	} else {
 		int64_t i;
-		if (sqlVdbeIntValue(pIn1, &i) != 0) {
+		bool is_unsigned = false;
+		if (sqlVdbeIntValue(pIn1, &i, &is_unsigned) != 0) {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 				 sql_value_text(pIn1), "integer");
 			rc = SQL_TARANTOOL_ERROR;
@@ -2481,7 +2488,8 @@ case OP_Or: {             /* same as TK_OR, in1, in2, out3 */
 		v2 = 2;
 	} else {
 		int64_t i;
-		if (sqlVdbeIntValue(pIn2, &i) != 0) {
+		bool is_unsigned = false;
+		if (sqlVdbeIntValue(pIn2, &i, &is_unsigned) != 0) {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 				 sql_value_text(pIn2), "integer");
 			rc = SQL_TARANTOOL_ERROR;
@@ -2519,7 +2527,8 @@ case OP_Not: {                /* same as TK_NOT, in1, out2 */
 	sqlVdbeMemSetNull(pOut);
 	if ((pIn1->flags & MEM_Null)==0) {
 		int64_t i;
-		if (sqlVdbeIntValue(pIn1, &i) != 0) {
+		bool is_unsigned = false;
+		if (sqlVdbeIntValue(pIn1, &i, &is_unsigned) != 0) {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 				 sql_value_text(pIn1), "integer");
 			rc = SQL_TARANTOOL_ERROR;
@@ -2544,7 +2553,8 @@ case OP_BitNot: {             /* same as TK_BITNOT, in1, out2 */
 	sqlVdbeMemSetNull(pOut);
 	if ((pIn1->flags & MEM_Null)==0) {
 		int64_t i;
-		if (sqlVdbeIntValue(pIn1, &i) != 0) {
+		bool is_unsigned = false;
+		if (sqlVdbeIntValue(pIn1, &i, &is_unsigned) != 0) {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 				 sql_value_text(pIn1), "integer");
 			rc = SQL_TARANTOOL_ERROR;
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index 2076a9a14..46094929f 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -490,7 +490,7 @@ void sqlVdbeMemSetNull(Mem *);
 void sqlVdbeMemSetZeroBlob(Mem *, int);
 int sqlVdbeMemMakeWriteable(Mem *);
 int sqlVdbeMemStringify(Mem *, u8);
-int sqlVdbeIntValue(Mem *, int64_t *);
+int sqlVdbeIntValue(Mem *, int64_t *, bool *);
 int sqlVdbeMemIntegerify(Mem *, bool is_forced);
 int sqlVdbeRealValue(Mem *, double *);
 int mem_apply_integer_type(Mem *);
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 06140569c..d8f9d9b87 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -215,7 +215,8 @@ int
 sql_value_int(sql_value * pVal)
 {
 	int64_t i;
-	sqlVdbeIntValue((Mem *) pVal, &i);
+	bool is_unsigned = false;
+	sqlVdbeIntValue((Mem *) pVal, &i, &is_unsigned);
 	return (int)i;
 }
 
@@ -223,7 +224,8 @@ sql_int64
 sql_value_int64(sql_value * pVal)
 {
 	int64_t i;
-	sqlVdbeIntValue((Mem *) pVal, &i);
+	bool is_unsigned = false;
+	sqlVdbeIntValue((Mem *) pVal, &i, &is_unsigned);
 	return i;
 }
 
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index 9e6d52b47..e4ea987cb 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -411,39 +411,36 @@ sqlVdbeMemRelease(Mem * p)
 }
 
 /*
- * Convert a 64-bit IEEE double into a 64-bit signed integer.
- * If the double is out of range of a 64-bit signed integer then
- * return the closest available 64-bit signed integer.
+ * Convert a 64-bit IEEE double into a 64-bit signed or unsigned integer.
+ * If the double is out of range of a 64-bit unsigned integer then
+ * return the closest available 64-bit unsigned integer.
+ * Returns 0 on success, -1 on error and 1 on precision loss.
  */
 static int
-doubleToInt64(double r, int64_t *i)
+doubleToInt64(double r, int64_t *i, bool* is_unsigned)
 {
-#ifdef SQL_OMIT_FLOATING_POINT
-	/* When floating-point is omitted, double and int64 are the same thing */
-	*i = r;
-	return 0;
-#else
-	/*
-	 * Many compilers we encounter do not define constants for the
-	 * minimum and maximum 64-bit integers, or they define them
-	 * inconsistently.  And many do not understand the "LL" notation.
-	 * So we define our own static constants here using nothing
-	 * larger than a 32-bit integer constant.
-	 */
-	static const int64_t maxInt = LARGEST_INT64;
-	static const int64_t minInt = SMALLEST_INT64;
+	static const int64_t minInt = INT64_MIN;
+	static const uint64_t maxUInt = UINT64_MAX;
+	static const int64_t maxInt = INT64_MAX;
 
-	if (r <= (double)minInt) {
+	if (r < (double)minInt){
 		*i = minInt;
+		*is_unsigned = false;
 		return -1;
-	} else if (r >= (double)maxInt) {
-		*i = maxInt;
+	} else if (r > (double)maxUInt) {
+		*i = maxUInt;
+		*is_unsigned = true;
 		return -1;
+	} else if (r > (double)maxInt){
+		uint64_t t = (uint64_t) r;
+		*i = (int64_t) t;
+		*is_unsigned = true;
+		return (t != r) ? 1 : 0;
 	} else {
 		*i = (int64_t) r;
-		return *i != r;
+		*is_unsigned = false;
+		return (*i != r) ? 1 : 0;
 	}
-#endif
 }
 
 /*
@@ -458,20 +455,30 @@ doubleToInt64(double r, int64_t *i)
  * If pMem represents a string value, its encoding might be changed.
  */
 int
-sqlVdbeIntValue(Mem * pMem, int64_t *i)
+sqlVdbeIntValue(Mem * pMem, int64_t *i, bool *is_unsigned)
 {
 	int flags;
 	assert(EIGHT_BYTE_ALIGNMENT(pMem));
 	flags = pMem->flags;
 	if (flags & MEM_Int) {
 		*i = pMem->u.i;
+		*is_unsigned = (flags & MEM_Unsigned) != 0;
 		return 0;
 	} else if (flags & MEM_Real) {
-		return doubleToInt64(pMem->u.r, i);
+		return doubleToInt64(pMem->u.r, i, is_unsigned);
 	} else if (flags & (MEM_Str)) {
 		assert(pMem->z || pMem->n == 0);
-		if (sql_atoi64(pMem->z, (int64_t *)i, pMem->n) == 0)
+		enum atoi_result rc = sql_atoi64(pMem->z, i, pMem->n);
+		switch(rc) {
+		case ATOI_SIGNED:
+			*is_unsigned = false;
 			return 0;
+		case ATOI_UNSIGNED:
+			*is_unsigned = true;
+			return 0;
+		default:
+			return -1;
+		}
 	}
 	return -1;
 }
@@ -514,9 +521,14 @@ mem_apply_integer_type(Mem *pMem)
 	assert(pMem->flags & MEM_Real);
 	assert(EIGHT_BYTE_ALIGNMENT(pMem));
 
-	if ((rc = doubleToInt64(pMem->u.r, (int64_t *) &ix)) == 0) {
+	bool is_unsigned = false;
+
+	if ((rc = doubleToInt64(pMem->u.r, (int64_t *) &ix, &is_unsigned)) == 0) {
 		pMem->u.i = ix;
-		MemSetTypeFlag(pMem, MEM_Int);
+		if (is_unsigned)
+			MemSetTypeFlag(pMem, MEM_Int | MEM_Unsigned);
+		else
+			MemSetTypeFlag(pMem, MEM_Int);
 	}
 	return rc;
 }
@@ -530,15 +542,23 @@ sqlVdbeMemIntegerify(Mem * pMem, bool is_forced)
 	assert(EIGHT_BYTE_ALIGNMENT(pMem));
 
 	int64_t i;
-	if (sqlVdbeIntValue(pMem, &i) == 0) {
+	bool is_unsigned = false;
+	if (sqlVdbeIntValue(pMem, &i, &is_unsigned) == 0) {
 		pMem->u.i = i;
-		MemSetTypeFlag(pMem, MEM_Int);
+
+		if (is_unsigned)
+			MemSetTypeFlag(pMem, MEM_Int | MEM_Unsigned);
+		else
+			MemSetTypeFlag(pMem, MEM_Int);
 		return 0;
 	} else if ((pMem->flags & MEM_Real) != 0 && is_forced) {
-		if (pMem->u.r >= INT64_MAX || pMem->u.r < INT64_MIN)
+		if (doubleToInt64(pMem->u.r, (int64_t*)&pMem->u.i, &is_unsigned) < 0)
 			return -1;
-		pMem->u.i = (int64_t) pMem->u.r;
-		MemSetTypeFlag(pMem, MEM_Int);
+
+		if (is_unsigned)
+			MemSetTypeFlag(pMem, MEM_Int | MEM_Unsigned);
+		else
+			MemSetTypeFlag(pMem, MEM_Int);
 		return 0;
 	}
 
diff --git a/test/sql/integer-overflow.result b/test/sql/integer-overflow.result
index aa9fabf1b..b6cacbeff 100644
--- a/test/sql/integer-overflow.result
+++ b/test/sql/integer-overflow.result
@@ -12,7 +12,7 @@ box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
 --
 box.sql.execute('SELECT (2147483647 * 2147483647 * 2147483647);')
 ---
-- error: 'Failed to execute SQL statement: integer is overflowed'
+- error: 'Failed to execute SQL statement: integer overflow'
 ...
 box.sql.execute('SELECT (-9223372036854775808 / -1);')
 ---
@@ -20,7 +20,7 @@ box.sql.execute('SELECT (-9223372036854775808 / -1);')
 ...
 box.sql.execute('SELECT (-9223372036854775808 - 1);')
 ---
-- error: 'Failed to execute SQL statement: integer is overflowed'
+- error: 'Failed to execute SQL statement: integer overflow'
 ...
 box.sql.execute('SELECT (9223372036854775807 + 1);')
 ---
-- 
2.17.1