[tarantool-patches] [PATCH 1/6] sql: refactor sql_atoi64()

[Nikita Pettik <korablev@tarantool.org>]

We are going to allow using unsigned values in SQL and extend range of
INTEGER type. Hence, we should be able to parse and operate on integers
in range of [2^63, 2^64 - 1]. Current mechanism which involves
sql_atoi64() function doesn't allow this.

Let's refactor this function: firstly, get rid of manual parsing and use
strtoll() and strtoull() functions from standard library. Then, let's
return sign of parsed literal. In case of success now function returns 0,
-1 otherwise.

This patch also inlines sql_dec_or_hex_to_i64() to place of its only
usage: it makes code cleaner and more straightforward.

Needed for #3810
Needed for #4015
---
 src/box/errcode.h      |   2 +-
 src/box/sql/expr.c     |  65 ++++++++++++++++-------
 src/box/sql/sqlInt.h   |  47 ++++-------------
 src/box/sql/sqlLimit.h |   9 ++++
 src/box/sql/util.c     | 140 +++++++++++--------------------------------------
 src/box/sql/vdbe.c     |   6 ++-
 src/box/sql/vdbemem.c  |  14 +++--
 7 files changed, 110 insertions(+), 173 deletions(-)

diff --git a/src/box/errcode.h b/src/box/errcode.h
index 9c15f3322..f4aba2f54 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -242,7 +242,7 @@ struct errcode_record {
 	/*187 */_(ER_SQL_ANALYZE_ARGUMENT,	"ANALYZE statement argument %s is not a base table") \
 	/*188 */_(ER_SQL_COLUMN_COUNT_MAX,	"Failed to create space '%s': space column count %d exceeds the limit (%d)") \
 	/*189 */_(ER_HEX_LITERAL_MAX,		"Hex literal %s%s length %d exceeds the supported limit (%d)") \
-	/*190 */_(ER_INT_LITERAL_MAX,		"Integer literal %s%s exceeds the supported range %lld - %lld") \
+	/*190 */_(ER_INT_LITERAL_MAX,		"Integer literal %s exceeds the supported range %lld - %lld") \
 	/*191 */_(ER_SQL_PARSER_LIMIT,		"%s %d exceeds the limit (%d)") \
 	/*192 */_(ER_INDEX_DEF_UNSUPPORTED,	"%s are prohibited in an index definition") \
 	/*193 */_(ER_CK_DEF_UNSUPPORTED,	"%s are prohibited in a CHECK constraint definition") \
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index ba7eea59d..158631416 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -1196,7 +1196,8 @@ sqlExprAssignVarNumber(Parse * pParse, Expr * pExpr, u32 n)
 			 * variable number
 			 */
 			int64_t i;
-			bool is_ok = 0 == sql_atoi64(&z[1], &i, n - 1);
+			bool is_neg;
+			bool is_ok = 0 == sql_atoi64(&z[1], &i, &is_neg, n - 1);
 			x = (ynVar) i;
 			testcase(i == 0);
 			testcase(i == 1);
@@ -3325,29 +3326,53 @@ expr_code_int(struct Parse *parse, struct Expr *expr, bool is_neg,
 		if (is_neg)
 			i = -i;
 		sqlVdbeAddOp2(v, OP_Integer, i, mem);
+		return;
+	}
+	int64_t value;
+	const char *z = expr->u.zToken;
+	assert(z != NULL);
+	const char *sign = is_neg ? "-" : "";
+	if (z[0] == '0' && (z[1] == 'x' || z[1] == 'X')) {
+		uint64_t u = 0;
+		int i, k;
+		for (i = 2; z[i] == '0'; i++);
+		for (k = i; sqlIsxdigit(z[k]); k++)
+			u = u * 16 + sqlHexToInt(z[k]);
+		memcpy(&value, &u, 8);
+		if (z[k] != 0 || k - i > 16) {
+			diag_set(ClientError, ER_HEX_LITERAL_MAX, sign, z,
+				 strlen(z) - 2, 16);
+			parse->is_aborted = true;
+			return;
+		}
 	} else {
-		int64_t value;
-		const char *z = expr->u.zToken;
-		assert(z != NULL);
-		int c = sql_dec_or_hex_to_i64(z, &value);
-		if (c == 1 || (c == 2 && !is_neg) ||
-		    (is_neg && value == SMALLEST_INT64)) {
-			const char *sign = is_neg ? "-" : "";
-			if (sql_strnicmp(z, "0x", 2) == 0) {
-				diag_set(ClientError, ER_HEX_LITERAL_MAX, sign,
-					 z, strlen(z) - 2, 16);
-			} else {
-				diag_set(ClientError, ER_INT_LITERAL_MAX, sign,
-					 z, INT64_MIN, INT64_MAX);
-			}
+		size_t len = strlen(z);
+		if (is_neg) {
+			/*
+			 * UINT64_MAX is constant, so each integer
+			 * literal certainly contains less digits.
+			 * It makes no sense to continue processing
+			 * string if it contains more characters.
+			 */
+			if (len > UINT64_MAX_DIGIT_COUNT)
+				goto int_overflow;
+			char *neg_z = tt_static_buf();
+			neg_z[0] = '-';
+			memcpy(neg_z + 1, z, len++);
+			neg_z[len]= '\0';
+			z = neg_z;
+		}
+		bool unused;
+		int rc = sql_atoi64(z, &value, &unused, len);
+		if (rc != 0) {
+int_overflow:
+			diag_set(ClientError, ER_INT_LITERAL_MAX, z, INT64_MIN,
+				 INT64_MAX);
 			parse->is_aborted = true;
-		} else {
-			if (is_neg)
-				value = c == 2 ? SMALLEST_INT64 : -value;
-			sqlVdbeAddOp4Dup8(v, OP_Int64, 0, mem, 0,
-					      (u8 *)&value, P4_INT64);
+			return;
 		}
 	}
+	sqlVdbeAddOp4Dup8(v, OP_Int64, 0, mem, 0, (u8 *)&value, P4_INT64);
 }
 
 /*
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 997a46535..da17e24ed 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4405,52 +4405,25 @@ field_type_sequence_dup(struct Parse *parse, enum field_type *types,
 			uint32_t len);
 
 /**
- * Convert z to a 64-bit signed integer.  z must be decimal. This
- * routine does *not* accept hexadecimal notation.
- *
- * If the z value is representable as a 64-bit twos-complement
- * integer, then write that value into *val and return 0.
- *
- * If z is exactly 9223372036854775808, return 2.  This special
- * case is broken out because while 9223372036854775808 cannot be
- * a signed 64-bit integer, its negative -9223372036854775808 can
- * be.
- *
- * If z is too big for a 64-bit integer and is not
- * 9223372036854775808  or if z contains any non-numeric text,
- * then return 1.
+ * Convert z to a 64-bit signed or unsigned integer.
+ * z must be decimal. This routine does *not* accept
+ * hexadecimal notation. Under the hood it calls
+ * strtoll or strtoull depending on presence of '-' char.
  *
  * length is the number of bytes in the string (bytes, not
  * characters). The string is not necessarily zero-terminated.
- * The encoding is given by enc.
  *
  * @param z String being parsed.
  * @param[out] val Output integer value.
+ * @param[out] is_neg Sign of the result.
  * @param length String length in bytes.
- * @retval
- *     0    Successful transformation.  Fits in a 64-bit signed
- *          integer.
- *     1    Integer too large for a 64-bit signed integer or is
- *          malformed
- *     2    Special case of 9223372036854775808
- */
-int
-sql_atoi64(const char *z, int64_t *val, int length);
-
-/**
- * Transform a UTF-8 integer literal, in either decimal or
- * hexadecimal, into a 64-bit signed integer.  This routine
- * accepts hexadecimal literals, whereas sql_atoi64() does not.
- *
- * @param z Literal being parsed.
- * @param[out] val Parsed value.
- * @retval
- *     0    Successful transformation.  Fits in a 64-bit signed integer.
- *     1    Integer too large for a 64-bit signed integer or is malformed
- *     2    Special case of 9223372036854775808
+ * @retval 0 Successful transformation. Fits in a 64-bit signed
+ *           or unsigned integer.
+ * @retval -1 Error during parsing: it contains non-digit
+ *            characters or it doesn't fit into 64-bit int.
  */
 int
-sql_dec_or_hex_to_i64(const char *z, int64_t *val);
+sql_atoi64(const char *z, int64_t *val, bool *is_neg, int length);
 
 void sqlErrorWithMsg(sql *, int, const char *, ...);
 void sqlError(sql *, int);
diff --git a/src/box/sql/sqlLimit.h b/src/box/sql/sqlLimit.h
index 53dbe1505..1c3038ed3 100644
--- a/src/box/sql/sqlLimit.h
+++ b/src/box/sql/sqlLimit.h
@@ -44,6 +44,15 @@ enum {
 	SQL_BIND_PARAMETER_MAX = 65000,
 };
 
+enum {
+	/**
+	 * Each unsigned (ergo signed) contains less than
+	 * 21 characters. It allows us to prevent from
+	 * parsing oversized literals and raise an error.
+	 */
+	UINT64_MAX_DIGIT_COUNT = 21
+};
+
 /*
  * The maximum length of a TEXT or BLOB in bytes.   This also
  * limits the size of a row in a table or index.
diff --git a/src/box/sql/util.c b/src/box/sql/util.c
index d5c93f8aa..2e7c298e1 100644
--- a/src/box/sql/util.c
+++ b/src/box/sql/util.c
@@ -593,120 +593,42 @@ sqlAtoF(const char *z, double *pResult, int length)
 	return z == zEnd && nDigits > 0 && eValid && nonNum == 0;
 }
 
-/*
- * Compare the 19-character string zNum against the text representation
- * value 2^63:  9223372036854775808.  Return negative, zero, or positive
- * if zNum is less than, equal to, or greater than the string.
- * Note that zNum must contain exactly 19 characters.
- *
- * Unlike memcmp() this routine is guaranteed to return the difference
- * in the values of the last digit if the only difference is in the
- * last digit.  So, for example,
- *
- *      compare2pow63("9223372036854775800", 1)
- *
- * will return -8.
- */
-static int
-compare2pow63(const char *zNum, int incr)
-{
-	int c = 0;
-	int i;
-	/* 012345678901234567 */
-	const char *pow63 = "922337203685477580";
-	for (i = 0; c == 0 && i < 18; i++) {
-		c = (zNum[i * incr] - pow63[i]) * 10;
-	}
-	if (c == 0) {
-		c = zNum[18 * incr] - '8';
-		testcase(c == (-1));
-		testcase(c == 0);
-		testcase(c == (+1));
-	}
-	return c;
-}
-
 int
-sql_atoi64(const char *z, int64_t *val, int length)
+sql_atoi64(const char *z, int64_t *val, bool *is_neg, int length)
 {
-	int incr = 1;
-	u64 u = 0;
-	int neg = 0;		/* assume positive */
-	int i;
-	int c = 0;
-	int nonNum = 0;		/* True if input contains UTF16 with high byte non-zero */
-	const char *zStart;
-	const char *zEnd = z + length;
-	incr = 1;
-	while (z < zEnd && sqlIsspace(*z))
-		z += incr;
-	if (z < zEnd) {
-		if (*z == '-') {
-			neg = 1;
-			z += incr;
-		} else if (*z == '+') {
-			z += incr;
-		}
-	}
-	zStart = z;
-	/* Skip leading zeros. */
-	while (z < zEnd && z[0] == '0') {
-		z += incr;
-	}
-	for (i = 0; &z[i] < zEnd && (c = z[i]) >= '0' && c <= '9';
-	     i += incr) {
-		u = u * 10 + c - '0';
-	}
-	if (u > LARGEST_INT64) {
-		*val = neg ? SMALLEST_INT64 : LARGEST_INT64;
-	} else if (neg) {
-		*val = -(i64) u;
+	*is_neg = false;
+	const char *str_end = z + length;
+	for (; z < str_end && sqlIsspace(*z); z++)
+	/* invalid format */
+	if (z >= str_end)
+		return -1;
+	if (*z == '-')
+		*is_neg = true;
+
+	char* end = NULL;
+	errno = 0;
+	if (*is_neg) {
+		*val = strtoll(z, &end, 10);
 	} else {
-		*val = (i64) u;
+		uint64_t u_val = strtoull(z, &end, 10);
+		if (u_val > INT64_MAX)
+			return -1;
+		*val = u_val;
 	}
-	if (&z[i] < zEnd || (i == 0 && zStart == z) || i > 19 * incr ||
-	    nonNum) {
-		/* zNum is empty or contains non-numeric text or is longer
-		 * than 19 digits (thus guaranteeing that it is too large)
-		 */
-		return 1;
-	} else if (i < 19 * incr) {
-		/* Less than 19 digits, so we know that it fits in 64 bits */
-		assert(u <= LARGEST_INT64);
-		return 0;
-	} else {
-		/* zNum is a 19-digit numbers.  Compare it against 9223372036854775808. */
-		c = compare2pow63(z, incr);
-		if (c < 0) {
-			/* zNum is less than 9223372036854775808 so it fits */
-			assert(u <= LARGEST_INT64);
-			return 0;
-		} else if (c > 0) {
-			/* zNum is greater than 9223372036854775808 so it overflows */
-			return 1;
-		} else {
-			/* zNum is exactly 9223372036854775808.  Fits if negative.  The
-			 * special case 2 overflow if positive
-			 */
-			assert(u - 1 == LARGEST_INT64);
-			return neg ? 0 : 2;
-		}
-	}
-}
+	/* No digits were found, e.g. an empty string. */
+	if (end == z)
+		return -1;
+	/*
+	 * String has been parsed, but there's
+	 * additional character(s) remained.
+	 */
+	if (*end != '\0')
+		return -1;
+	/* Overflow and underflow errors. */
+	if (errno != 0)
+		return -1;
 
-int
-sql_dec_or_hex_to_i64(const char *z, int64_t *val)
-{
-	if (z[0] == '0' && (z[1] == 'x' || z[1] == 'X')) {
-		uint64_t u = 0;
-		int i, k;
-		for (i = 2; z[i] == '0'; i++);
-		for (k = i; sqlIsxdigit(z[k]); k++)
-			u = u * 16 + sqlHexToInt(z[k]);
-		memcpy(val, &u, 8);
-		return (z[k] == 0 && k - i <= 16) ? 0 : 1;
-	}
-	return sql_atoi64(z, val, sqlStrlen30(z));
+	return 0;
 }
 
 /*
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index d083d3709..6ecdb26fc 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -279,7 +279,8 @@ mem_apply_numeric_type(struct Mem *record)
 	if ((record->flags & (MEM_Str | MEM_Int | MEM_Real)) != MEM_Str)
 		return -1;
 	int64_t integer_value;
-	if (sql_atoi64(record->z, &integer_value, record->n) == 0) {
+	bool is_neg;
+	if (sql_atoi64(record->z, &integer_value, &is_neg, record->n) == 0) {
 		record->u.i = integer_value;
 		MemSetTypeFlag(record, MEM_Int);
 		return 0;
@@ -384,7 +385,8 @@ static u16 SQL_NOINLINE computeNumericType(Mem *pMem)
 	assert((pMem->flags & (MEM_Str|MEM_Blob))!=0);
 	if (sqlAtoF(pMem->z, &pMem->u.r, pMem->n)==0)
 		return 0;
-	if (sql_atoi64(pMem->z, (int64_t *)&pMem->u.i, pMem->n)==SQL_OK)
+	bool is_neg;
+	if (sql_atoi64(pMem->z, (int64_t *)&pMem->u.i, &is_neg, pMem->n)==SQL_OK)
 		return MEM_Int;
 	return MEM_Real;
 }
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index 08b649926..3a361d066 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -466,7 +466,8 @@ sqlVdbeIntValue(Mem * pMem, int64_t *i)
 		return doubleToInt64(pMem->u.r, i);
 	} else if (flags & (MEM_Str)) {
 		assert(pMem->z || pMem->n == 0);
-		if (sql_atoi64(pMem->z, (int64_t *)i, pMem->n) == 0)
+		bool is_neg;
+		if (sql_atoi64(pMem->z, (int64_t *)i, &is_neg, pMem->n) == 0)
 			return 0;
 	}
 	return -1;
@@ -586,7 +587,9 @@ sqlVdbeMemNumerify(Mem * pMem)
 {
 	if ((pMem->flags & (MEM_Int | MEM_Real | MEM_Null)) == 0) {
 		assert((pMem->flags & (MEM_Blob | MEM_Str)) != 0);
-		if (0 == sql_atoi64(pMem->z, (int64_t *)&pMem->u.i, pMem->n)) {
+		bool is_neg;
+		if (0 == sql_atoi64(pMem->z, (int64_t *)&pMem->u.i, &is_neg,
+				    pMem->n)) {
 			MemSetTypeFlag(pMem, MEM_Int);
 		} else {
 			double v;
@@ -651,7 +654,9 @@ sqlVdbeMemCast(Mem * pMem, enum field_type type)
 	if (pMem->flags & MEM_Null)
 		return SQL_OK;
 	if ((pMem->flags & MEM_Blob) != 0 && type == FIELD_TYPE_NUMBER) {
-		if (sql_atoi64(pMem->z, (int64_t *) &pMem->u.i, pMem->n) == 0) {
+		bool is_neg;
+		if (sql_atoi64(pMem->z, (int64_t *) &pMem->u.i, &is_neg,
+			       pMem->n) == 0) {
 			MemSetTypeFlag(pMem, MEM_Real);
 			pMem->u.r = pMem->u.i;
 			return 0;
@@ -682,7 +687,8 @@ sqlVdbeMemCast(Mem * pMem, enum field_type type)
 		return -1;
 	case FIELD_TYPE_INTEGER:
 		if ((pMem->flags & MEM_Blob) != 0) {
-			if (sql_atoi64(pMem->z, (int64_t *) &pMem->u.i,
+			bool is_neg;
+			if (sql_atoi64(pMem->z, (int64_t *) &pMem->u.i, &is_neg,
 				       pMem->n) != 0)
 				return -1;
 			MemSetTypeFlag(pMem, MEM_Int);
-- 
2.15.1