diff --git a/src/box/sql/func.c b/src/box/sql/func.c index df89e89e0d9b34ecb5ec6a88ad2bc339798cd783..d655bcf688784948796cd4824eed525bf4a6d9d6 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -1535,7 +1535,9 @@ soundexFunc(sql_context * context, int argc, sql_value ** argv) typedef struct SumCtx SumCtx; struct SumCtx { double rSum; /* Floating point sum */ - i64 iSum; /* Integer sum */ + int64_t iSum; /* Integer sum */ + /** True if iSum < 0. */ + bool is_neg; i64 cnt; /* Number of elements summed */ u8 overflow; /* True if integer overflow seen */ u8 approx; /* True if non-integer value was input to the sum */ @@ -1572,9 +1574,13 @@ sum_step(struct sql_context *context, int argc, sql_value **argv) p->cnt++; if (type == MP_INT || type == MP_UINT) { int64_t v = sql_value_int64(argv[0]); - p->rSum += v; + if (type == MP_INT) + p->rSum += v; + else + p->rSum += (uint64_t) v; if ((p->approx | p->overflow) == 0 && - sqlAddInt64(&p->iSum, v) != 0) { + sql_add_int(p->iSum, p->is_neg, v, type == MP_INT, &p->iSum, + &p->is_neg) != 0) { p->overflow = 1; } } else { @@ -1596,7 +1602,7 @@ sumFinalize(sql_context * context) } else if (p->approx) { sql_result_double(context, p->rSum); } else { - sql_result_int64(context, p->iSum); + mem_set_int(context->pOut, p->iSum, p->is_neg); } } } diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index c78753246c4847f4fa034a297f3e77011922b49f..ac17117d2240b57cd4af6e25a2740fef0191b565 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -4034,9 +4034,37 @@ Expr *sqlExprAddCollateString(Parse *, Expr *, const char *); Expr *sqlExprSkipCollate(Expr *); int sqlCheckIdentifierName(Parse *, char *); void sqlVdbeSetChanges(sql *, int); -int sqlAddInt64(i64 *, i64); -int sqlSubInt64(i64 *, i64); -int sqlMulInt64(i64 *, i64); + +/** + * Attempt to add, subtract, multiply or get the remainder of + * 64-bit integer values. There functions are able to operate + * on signed as well as unsigned integers. If result of operation + * is greater 0, then it is assumed to be unsigned and can take + * values in range up to 2^64 - 1. If the result is negative, + * then its minimum value is -2^63. + * Return 0 on success. Or if the operation would have resulted + * in an overflow, return -1. + */ +int +sql_add_int(int64_t lhs, bool is_lhs_neg, int64_t rhs, bool is_rhs_neg, + int64_t *res, bool *is_res_neg); + +int +sql_sub_int(int64_t lhs, bool is_lhs_neg, int64_t rhs, bool is_rhs_neg, + int64_t *res, bool *is_res_neg); + +int +sql_mul_int(int64_t lhs, bool is_lhs_neg, int64_t rhs, bool is_rhs_neg, + int64_t *res, bool *is_res_neg); + +int +sql_div_int(int64_t lhs, bool is_lhs_neg, int64_t rhs, bool is_rhs_neg, + int64_t *res, bool *is_res_neg); + +int +sql_rem_int(int64_t lhs, bool is_lhs_neg, int64_t rhs, bool is_rhs_neg, + int64_t *res, bool *is_res_neg); + u8 sqlGetBoolean(const char *z, u8); const void *sqlValueText(sql_value *); diff --git a/src/box/sql/util.c b/src/box/sql/util.c index ce4e3a6c6357540d3bccc8620af13ef3deda343a..161c1f6071320ca0d2ba30042933a9379fbb49a0 100644 --- a/src/box/sql/util.c +++ b/src/box/sql/util.c @@ -945,74 +945,178 @@ sqlHexToBlob(sql * db, const char *z, int n) } #endif /* !SQL_OMIT_BLOB_LITERAL || SQL_HAS_CODEC */ -/* - * Attempt to add, substract, or multiply the 64-bit signed value iB against - * the other 64-bit signed integer at *pA and store the result in *pA. - * Return 0 on success. Or if the operation would have resulted in an - * overflow, leave *pA unchanged and return 1. - */ int -sqlAddInt64(i64 * pA, i64 iB) +sql_add_int(int64_t lhs, bool is_lhs_neg, int64_t rhs, bool is_rhs_neg, + int64_t *res, bool *is_res_neg) { - i64 iA = *pA; - testcase(iA == 0); - testcase(iA == 1); - testcase(iB == -1); - testcase(iB == 0); - if (iB >= 0) { - testcase(iA > 0 && LARGEST_INT64 - iA == iB); - testcase(iA > 0 && LARGEST_INT64 - iA == iB - 1); - if (iA > 0 && LARGEST_INT64 - iA < iB) - return 1; - } else { - testcase(iA < 0 && -(iA + LARGEST_INT64) == iB + 1); - testcase(iA < 0 && -(iA + LARGEST_INT64) == iB + 2); - if (iA < 0 && -(iA + LARGEST_INT64) > iB + 1) - return 1; + /* Addition of two negative integers. */ + if (is_lhs_neg && is_rhs_neg) { + assert(lhs < 0 && rhs < 0); + if (lhs < INT64_MIN - rhs) + return -1; + *is_res_neg = true; + *res = lhs + rhs; + return 0; } - *pA += iB; + /* Both are unsigned integers. */ + if (!is_lhs_neg && !is_rhs_neg) { + uint64_t u_lhs = (uint64_t) lhs; + uint64_t u_rhs = (uint64_t) rhs; + if (UINT64_MAX - u_lhs < u_rhs) + return -1; + *is_res_neg = false; + *res = lhs + rhs; + return 0; + } + *is_res_neg = is_rhs_neg ? (uint64_t)(-rhs) > (uint64_t) lhs : + (uint64_t)(-lhs) > (uint64_t) rhs; + *res = lhs + rhs; return 0; } int -sqlSubInt64(i64 * pA, i64 iB) +sql_sub_int(int64_t lhs, bool is_lhs_neg, int64_t rhs, bool is_rhs_neg, + int64_t *res, bool *is_res_neg) { - testcase(iB == SMALLEST_INT64 + 1); - if (iB == SMALLEST_INT64) { - testcase((*pA) == (-1)); - testcase((*pA) == 0); - if ((*pA) >= 0) - return 1; - *pA -= iB; + if (!is_lhs_neg && !is_rhs_neg) { + uint64_t u_lhs = (uint64_t) lhs; + uint64_t u_rhs = (uint64_t) rhs; + if (u_lhs >= u_rhs) { + *is_res_neg = false; + *res = u_lhs - u_rhs; + return 0; + } + if (u_rhs - u_lhs > (uint64_t) INT64_MAX + 1) + return -1; + *is_res_neg = true; + *res = lhs - rhs; return 0; - } else { - return sqlAddInt64(pA, -iB); } + if (is_rhs_neg) { + return sql_add_int(lhs, is_lhs_neg, -rhs, false, res, + is_res_neg); + } + assert(is_lhs_neg && !is_rhs_neg); + /* + * (lhs - rhs) < 0, lhs < 0, rhs > 0: in this case their + * difference must not be less than INT64_MIN. + */ + if ((uint64_t) -lhs + (uint64_t) rhs > (uint64_t) INT64_MAX + 1) + return -1; + *is_res_neg = true; + *res = lhs - rhs; + return 0; } int -sqlMulInt64(i64 * pA, i64 iB) +sql_mul_int(int64_t lhs, bool is_lhs_neg, int64_t rhs, bool is_rhs_neg, + int64_t *res, bool *is_res_neg) { - i64 iA = *pA; - if (iB > 0) { - if (iA > LARGEST_INT64 / iB) - return 1; - if (iA < SMALLEST_INT64 / iB) - return 1; - } else if (iB < 0) { - if (iA > 0) { - if (iB < SMALLEST_INT64 / iA) - return 1; - } else if (iA < 0) { - if (iB == SMALLEST_INT64) - return 1; - if (iA == SMALLEST_INT64) - return 1; - if (-iA > LARGEST_INT64 / -iB) - return 1; - } + if (lhs == 0 || rhs == 0) { + *res = 0; + *is_res_neg = false; + return 0; + } + /* + * Multiplication of integers with same sign leads to + * unsigned result. + */ + if (is_lhs_neg == is_rhs_neg) { + uint64_t u_res = is_lhs_neg ? + (uint64_t) (-lhs) * (uint64_t) (-rhs) : + (uint64_t) lhs * (uint64_t) (rhs); + /* + * Overflow detection is quite primitive due to + * the absence of overflow with unsigned values: + * lhs * rhs == res --> rhs == res / lhs; + * If this predicate is false, then result was + * reduced modulo UINT_MAX + 1. + */ + if ((is_lhs_neg && u_res / (uint64_t) (-lhs) != + (uint64_t) (-rhs)) || + (!is_lhs_neg && u_res / (uint64_t) lhs != (uint64_t) rhs)) + return -1; + *is_res_neg = false; + *res = u_res; + return 0; + } + /* + * Make sure we've got only one combination of + * positive and negative operands. + */ + if (is_lhs_neg) { + SWAP(is_lhs_neg, is_rhs_neg); + SWAP(lhs, rhs); + } + assert(! is_lhs_neg && is_rhs_neg); + uint64_t u_rhs = (uint64_t) (-rhs); + uint64_t u_res = u_rhs * (uint64_t) lhs; + if (u_res / u_rhs != (uint64_t) lhs || + u_res > (uint64_t) INT64_MAX + 1) + return -1; + *is_res_neg = true; + *res = lhs * rhs; + return 0; +} + +int +sql_div_int(int64_t lhs, bool is_lhs_neg, int64_t rhs, bool is_rhs_neg, + int64_t *res, bool *is_res_neg) +{ + if (lhs == 0) { + *res = 0; + *is_res_neg = false; + return 0; + } + /* + * The only possible overflow situations is when operands + * of different signs and result turns out to be less + * than INT64_MIN. + */ + if (is_lhs_neg != is_rhs_neg) { + uint64_t u_res = is_lhs_neg ? + (uint64_t) (-lhs) / (uint64_t) rhs : + (uint64_t) lhs / (uint64_t) (-rhs); + if (u_res > (uint64_t) INT64_MAX + 1) + return -1; + *is_res_neg = u_res != 0; + *res = -u_res; + return 0; + } + *is_res_neg = false; + /* + * Another one special case: INT64_MIN / -1 + * Signed division leads to program termination due + * to overflow. + */ + if (is_lhs_neg && lhs == INT64_MIN && rhs == -1) { + *res = (uint64_t) INT64_MAX + 1; + return 0; + } + *res = is_lhs_neg ? (uint64_t) (lhs / rhs) : + (uint64_t) lhs / (uint64_t) rhs; + return 0; +} + +int +sql_rem_int(int64_t lhs, bool is_lhs_neg, int64_t rhs, bool is_rhs_neg, + int64_t *res, bool *is_res_neg) +{ + uint64_t u_rhs = is_rhs_neg ? (uint64_t) (-rhs) : (uint64_t) rhs; + if (is_lhs_neg) { + uint64_t u_lhs = (uint64_t) (-lhs); + uint64_t u_res = u_lhs % u_rhs; + *res = -u_res; + *is_res_neg = true; + return 0; } - *pA = iA * iB; + /* + * While calculating remainder we always ignore sign of + * rhs - it doesn't affect the result. + * */ + uint64_t u_lhs = (uint64_t) lhs; + *res = u_lhs % u_rhs; + *is_res_neg = false; return 0; } diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index ddf84064a51ffb5b42def19eac4c46ab79ff2f65..7f8db4dee6d7406eba424dbcc412cc1618b2ef63 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -1597,27 +1597,48 @@ case OP_Remainder: { /* same as TK_REM, in1, in2, out3 */ (type2 & (MEM_Int | MEM_UInt)) != 0) { iA = pIn1->u.i; iB = pIn2->u.i; + 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 (sqlAddInt64(&iB,iA)) goto integer_overflow; break; - case OP_Subtract: if (sqlSubInt64(&iB,iA)) goto integer_overflow; break; - case OP_Multiply: if (sqlMulInt64(&iB,iA)) goto integer_overflow; break; + case OP_Add: { + if (sql_add_int(iA, is_lhs_neg, iB, is_rhs_neg, + (int64_t *) &iB, &is_res_neg) != 0) + goto integer_overflow; + break; + } + case OP_Subtract: { + if (sql_sub_int(iB, is_rhs_neg, iA, is_lhs_neg, + (int64_t *) &iB, &is_res_neg) != 0) + goto integer_overflow; + break; + } + case OP_Multiply: { + if (sql_mul_int(iA, is_lhs_neg, iB, is_rhs_neg, + (int64_t *) &iB, &is_res_neg) != 0) + goto integer_overflow; + break; + } case OP_Divide: { if (iA == 0) goto division_by_zero; - if (iA==-1 && iB==SMALLEST_INT64) goto integer_overflow; - iB /= iA; + if (sql_div_int(iB, is_rhs_neg, iA, is_lhs_neg, + (int64_t *) &iB, &is_res_neg) != 0) + goto integer_overflow; break; } default: { if (iA == 0) goto division_by_zero; if (iA==-1) iA = 1; - iB %= iA; + if (sql_rem_int(iB, is_rhs_neg, iA, is_lhs_neg, + (int64_t *) &iB, &is_res_neg) != 0) + goto integer_overflow; break; } } - mem_set_i64(pOut, iB); + mem_set_int(pOut, iB, is_res_neg); } else { bIntint = 0; if (sqlVdbeRealValue(pIn1, &rA) != 0) { @@ -1843,12 +1864,12 @@ case OP_ShiftRight: { /* same as TK_RSHIFT, in1, in2, out3 */ break; } bool unused; - if (sqlVdbeIntValue(pIn2, &iA, &unused) != 0) { + if (sqlVdbeIntValue(pIn2, (int64_t *) &iA, &unused) != 0) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, sql_value_text(pIn2), "integer"); goto abort_due_to_error; } - if (sqlVdbeIntValue(pIn1, &iB, &unused) != 0) { + if (sqlVdbeIntValue(pIn1, (int64_t *) &iB, &unused) != 0) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, sql_value_text(pIn1), "integer"); goto abort_due_to_error; @@ -2144,11 +2165,31 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ /* Handle the common case of integer comparison here, as an * optimization, to avoid a call to sqlMemCompare() */ - if ((pIn1->flags & pIn3->flags & - (MEM_Int | MEM_UInt)) != 0) { - if (pIn3->u.i > pIn1->u.i) { res = +1; goto compare_op; } - if (pIn3->u.i < pIn1->u.i) { res = -1; goto compare_op; } - res = 0; + if ((pIn1->flags & pIn3->flags & (MEM_Int | MEM_UInt)) != 0) { + if ((pIn1->flags & pIn3->flags & MEM_Int) != 0) { + if (pIn3->u.i > pIn1->u.i) + res = +1; + else if (pIn3->u.i < pIn1->u.i) + res = -1; + else + res = 0; + goto compare_op; + } + if ((pIn1->flags & pIn3->flags & MEM_UInt) != 0) { + if (pIn3->u.u > pIn1->u.u) + res = +1; + else if (pIn3->u.u < pIn1->u.u) + res = -1; + else + res = 0; + goto compare_op; + } + if ((pIn1->flags & MEM_UInt) != 0 && + (pIn3->flags & MEM_Int) != 0) { + res = -1; + goto compare_op; + } + res = 1; goto compare_op; } } else if (type == FIELD_TYPE_STRING) { @@ -4916,7 +4957,9 @@ case OP_OffsetLimit: { /* in1, out2, in3 */ assert((pIn1->flags & MEM_UInt) != 0); assert((pIn3->flags & MEM_UInt) != 0); uint64_t x = pIn1->u.u; - if (sqlAddInt64((i64 *) &x, pIn3->u.u) != 0) { + uint64_t rhs = pIn3->u.u; + bool unused; + if (sql_add_int(x, false, rhs, false, (int64_t *) &x, &unused) != 0) { diag_set(ClientError, ER_SQL_EXECUTE, "sum of LIMIT and OFFSET " "values should not result in integer overflow"); goto abort_due_to_error; diff --git a/test/sql-tap/func.test.lua b/test/sql-tap/func.test.lua index 0f14759cf533a0ef1d500e4e14506ecd5f12ce5b..e132262425687a7e1b8f9733675140d617918b06 100755 --- a/test/sql-tap/func.test.lua +++ b/test/sql-tap/func.test.lua @@ -1,6 +1,6 @@ #!/usr/bin/env tarantool test = require("sqltester") -test:plan(14602) +test:plan(14603) --!./tcltestrunner.lua -- 2001 September 15 @@ -912,25 +912,25 @@ test:do_execsql_test( -- </func-8.6> }) -test:do_catchsql_test( +test:do_execsql_test( "func-8.7", [[ SELECT typeof(sum(x)) FROM (SELECT '9223372036' || '854775808' AS x UNION ALL SELECT -9223372036854775807) ]], { -- <func-8.7> - 1, "Failed to execute SQL statement: integer overflow" + "integer" -- </func-8.7> }) -test:do_catchsql_test( +test:do_execsql_test( "func-8.8", [[ SELECT sum(x)>0.0 FROM (SELECT '9223372036' || '854775808' AS x UNION ALL SELECT -9223372036850000000) ]], { -- <func-8.8> - 1, "Failed to execute SQL statement: integer overflow" + true -- </func-8.8> }) @@ -1591,14 +1591,14 @@ test:do_execsql_test( -- </func-18.11> }) -test:do_catchsql_test( +test:do_execsql_test( "func-18.12", [[ INSERT INTO t6 VALUES(3, 1<<62); SELECT sum(x) - ((1<<62)*2.0+1) from t6; ]], { -- <func-18.12> - 1, "Failed to execute SQL statement: integer overflow" + 0 -- </func-18.12> }) @@ -1645,18 +1645,30 @@ test:do_catchsql_test( -- </func-18.17> }) -test:do_catchsql_test( - "func-18.15", +test:do_execsql_test( + "func-18.15.1", [[ SELECT sum(x) FROM (SELECT 9223372036854775807 AS x UNION ALL SELECT 10 AS x); ]], { - -- <func-18.15> - 1, "Failed to execute SQL statement: integer overflow" - -- </func-18.15> + -- <func-18.15.1> + 9223372036854775817LL + -- </func-18.15.1> }) +test:do_catchsql_test( + "func-18.15.2", + [[ + SELECT sum(x) FROM + (SELECT 9223372036854775807 AS x UNION ALL SELECT 9223372036854775807 AS x + UNION ALL SELECT 10 AS x); + ]], { + -- <func-18.15.2> + 1, "Failed to execute SQL statement: integer overflow" + -- </func-18.15.2> +}) + test:do_catchsql_test( "func-18.18", [[ diff --git a/test/sql-tap/limit.test.lua b/test/sql-tap/limit.test.lua index 97bde6abe30506e82356393c5411a47c4ddff1de..f6866151c414920769dd532910de247d374874c2 100755 --- a/test/sql-tap/limit.test.lua +++ b/test/sql-tap/limit.test.lua @@ -1360,12 +1360,12 @@ test:do_catchsql_test( "limit-15.1", [[ SELECT 1 LIMIT 9223372036854775807 OFFSET 1; - ]], { 1, "Failed to execute SQL statement: sum of LIMIT and OFFSET values should not result in integer overflow" } ) + ]], { 0, {} } ) test:do_catchsql_test( "limit-15.2", [[ SELECT 1 LIMIT 1 OFFSET 9223372036854775807; - ]], { 1, "Failed to execute SQL statement: sum of LIMIT and OFFSET values should not result in integer overflow"} ) + ]], { 0, {} } ) test:finish_test() diff --git a/test/sql/integer-overflow.result b/test/sql/integer-overflow.result index 13914ad7b1303236135f6c1e36af257944ee72ef..6d88f16c93499638f16d04a44a0c828334a44367 100644 --- a/test/sql/integer-overflow.result +++ b/test/sql/integer-overflow.result @@ -10,6 +10,7 @@ box.execute('pragma sql_default_engine=\''..engine..'\'') ... -- gh-3735: make sure that integer overflows errors are -- handled during VDBE execution. +-- gh-3810: range of integer is extended up to 2^64 - 1. -- box.execute('SELECT (2147483647 * 2147483647 * 2147483647);') --- @@ -17,7 +18,11 @@ box.execute('SELECT (2147483647 * 2147483647 * 2147483647);') ... box.execute('SELECT (-9223372036854775808 / -1);') --- -- error: 'Failed to execute SQL statement: integer is overflowed' +- metadata: + - name: (-9223372036854775808 / -1) + type: integer + rows: + - [9223372036854775808] ... box.execute('SELECT (-9223372036854775808 - 1);') --- @@ -25,6 +30,14 @@ box.execute('SELECT (-9223372036854775808 - 1);') ... box.execute('SELECT (9223372036854775807 + 1);') --- +- metadata: + - name: (9223372036854775807 + 1) + type: integer + rows: + - [9223372036854775808] +... +box.execute('SELECT (9223372036854775807 + 9223372036854775807 + 2);') +--- - error: 'Failed to execute SQL statement: integer is overflowed' ... -- Literals are checked right after parsing. diff --git a/test/sql/integer-overflow.test.lua b/test/sql/integer-overflow.test.lua index 4339edf39c0fb29a5a8e6ea7b5a4dc7547d12bf8..7727f368cdee574abc18a5aba0c8b5a2a4e75936 100644 --- a/test/sql/integer-overflow.test.lua +++ b/test/sql/integer-overflow.test.lua @@ -4,11 +4,14 @@ box.execute('pragma sql_default_engine=\''..engine..'\'') -- gh-3735: make sure that integer overflows errors are -- handled during VDBE execution. +-- gh-3810: range of integer is extended up to 2^64 - 1. -- box.execute('SELECT (2147483647 * 2147483647 * 2147483647);') box.execute('SELECT (-9223372036854775808 / -1);') box.execute('SELECT (-9223372036854775808 - 1);') box.execute('SELECT (9223372036854775807 + 1);') +box.execute('SELECT (9223372036854775807 + 9223372036854775807 + 2);') + -- Literals are checked right after parsing. -- box.execute('SELECT 9223372036854775808;')