[Tarantool-patches] [PATCH v3 2/2] sql: allow to convert big real values to integer
imeevma at tarantool.org
imeevma at tarantool.org
Mon Dec 9 16:34:16 MSK 2019
Hi! Thank you for review! My answers and new patch below.
On 12/5/19 3:36 PM, Nikita Pettik wrote:
> On 23 Nov 14:48, Mergen Imeev wrote:
>>> BTW below I see path which is executed when
>>> is_forced == false. And it is also about same wrong conversion. Please
>>> check it as well and add test case.
>>>
>> I found that is_forced variable did nothing it this code.
>> Removed it from code along with "if" checked it.
>
> If it is unused, please move this refactoring to a separate patch.
>
Done.
>> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
>> index ab86be9..450e21d 100644
>> --- a/src/box/sql/vdbe.c
>> +++ b/src/box/sql/vdbe.c
>> @@ -322,13 +322,16 @@ mem_apply_type(struct Mem *record, enum field_type type)
>> if ((record->flags & MEM_UInt) == MEM_UInt)
>> return 0;
>> if ((record->flags & MEM_Real) == MEM_Real) {
>> - int64_t i = (int64_t) record->u.r;
>> - if (i == record->u.r)
>> - mem_set_int(record, record->u.r,
>> - record->u.r <= -1);
>> + double d = record->u.r;
>> + int64_t i = (int64_t) d;
>> + uint64_t u = (uint64_t) d;
>> + if (i == d)
>> + mem_set_int(record, d, d <= -1);
>
> Nit: mem_set_int(record, i, i <= -1;);
>
Fixed.
>> + else if (u == d)
>> + mem_set_u64(record, d);
>
> Nit: mem_set_u64(record, u);
>
Fixed.
New patch:
>From b4e446e1e7a0309e2e20ebdef84feff73805bb59 Mon Sep 17 00:00:00 2001
From: Mergen Imeev <imeevma at gmail.com>
Date: Tue, 5 Nov 2019 20:17:52 +0300
Subject: [PATCH] sql: allow to convert big real values to integer
This patch fixes a bug that prevented the conversion of real
values that are greater than INT64_MAX and less than UINT64_MAX to
INTEGER and UNSIGNED.
Closes #4526
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 8ea1df8..6ebc5f0 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -322,10 +322,13 @@ mem_apply_type(struct Mem *record, enum field_type type)
if ((record->flags & MEM_UInt) == MEM_UInt)
return 0;
if ((record->flags & MEM_Real) == MEM_Real) {
- int64_t i = (int64_t) record->u.r;
- if (i == record->u.r)
- mem_set_int(record, record->u.r,
- record->u.r <= -1);
+ double d = record->u.r;
+ int64_t i = (int64_t) d;
+ uint64_t u = (uint64_t) d;
+ if (i == d)
+ mem_set_int(record, i, i <= -1);
+ else if (u == d)
+ mem_set_u64(record, u);
return 0;
}
if (sqlVdbeMemIntegerify(record) != 0)
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index e2e2b7a..407b42e 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -568,10 +568,15 @@ sqlVdbeMemIntegerify(struct Mem *pMem)
double d;
if (sqlVdbeRealValue(pMem, &d) != 0)
return -1;
- if (d >= INT64_MAX || d < INT64_MIN)
- return -1;
- mem_set_int(pMem, d, d <= -1);
- return 0;
+ if (d < INT64_MAX && d >= INT64_MIN) {
+ mem_set_int(pMem, d, d <= -1);
+ return 0;
+ }
+ if (d >= INT64_MAX && d < UINT64_MAX) {
+ mem_set_u64(pMem, d);
+ return 0;
+ }
+ return -1;
}
/*
diff --git a/test/sql-tap/numcast.test.lua b/test/sql-tap/numcast.test.lua
index 9f72485..07117d0 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(11)
+test:plan(20)
--!./tcltestrunner.lua
-- 2013 March 20
@@ -65,5 +65,86 @@ for _, enc in ipairs({"utf8"}) do
end
end
+--
+-- gh-4526: make sure that DOUBLE values that more than
+-- 9223372036854775296 and less than 18446744073709551616 can be
+-- converted to INTEGER or UNSIGNED.
+--
+test:do_execsql_test(
+ "cast-2.1",
+ [[
+ SELECT CAST((9223372036854775297.01) AS INTEGER);
+ ]], {
+ 9223372036854775808ULL
+ })
+
+test:do_execsql_test(
+ "cast-2.2",
+ [[
+ SELECT CAST((18000000000000000000.) AS INTEGER);
+ ]], {
+ 18000000000000000000ULL
+ })
+
+test:do_execsql_test(
+ "cast-2.3",
+ [[
+ SELECT CAST((9223372036854775297.01) AS UNSIGNED);
+ ]], {
+ 9223372036854775808ULL
+ })
+
+test:do_execsql_test(
+ "cast-2.4",
+ [[
+ SELECT CAST((18000000000000000000.) AS UNSIGNED);
+ ]], {
+ 18000000000000000000ULL
+ })
+
+test:do_catchsql_test(
+ "cast-2.5",
+ [[
+ SELECT CAST((20000000000000000000.) AS UNSIGNED);
+ ]], {
+ 1,"Type mismatch: can not convert 2.0e+19 to unsigned"
+ })
+
+test:do_execsql_test(
+ "cast-2.6",
+ [[
+ CREATE TABLE t (i INTEGER PRIMARY KEY);
+ INSERT INTO t VALUES(9223372036854775297.01);
+ SELECT * FROM t;
+ ]], {
+ 9223372036854775808ULL
+ })
+
+test:do_execsql_test(
+ "cast-2.7",
+ [[
+ INSERT INTO t VALUES(18000000000000000000.01);
+ SELECT * FROM t;
+ ]], {
+ 9223372036854775808ULL,18000000000000000000ULL
+ })
+
+test:do_catchsql_test(
+ "cast-2.8",
+ [[
+ INSERT INTO t VALUES(20000000000000000000.01);
+ SELECT * FROM t;
+ ]], {
+ 1,"Tuple field 1 type does not match one required by operation: expected integer"
+ })
+
+test:do_catchsql_test(
+ "cast-2.9",
+ [[
+ INSERT INTO t VALUES(2.1);
+ SELECT * FROM t;
+ ]], {
+ 1,"Tuple field 1 type does not match one required by operation: expected integer"
+ })
test:finish_test()
diff --git a/test/sql/integer-overflow.result b/test/sql/integer-overflow.result
index 223ba02..d6223ff 100644
--- a/test/sql/integer-overflow.result
+++ b/test/sql/integer-overflow.result
@@ -110,15 +110,15 @@ box.execute('SELECT CAST(\'18446744073709551616\' AS INTEGER);')
- 'Type mismatch: can not convert 18446744073709551616 to integer'
...
-- Due to inexact represantation of large integers in terms of
--- floating point numbers, numerics with value < INT64_MAX
--- have INT64_MAX + 1 value in integer representation:
--- float 9223372036854775800 -> int (9223372036854775808),
--- with error due to conversion = 8.
+-- floating point numbers, numerics with value < UINT64_MAX
+-- have UINT64_MAX + 1 value in integer representation:
+-- float 18446744073709551600 -> int (18446744073709551616),
+-- with error due to conversion = 16.
--
-box.execute('SELECT CAST(9223372036854775807.0 AS INTEGER);')
+box.execute('SELECT CAST(18446744073709551600. AS INTEGER);')
---
- null
-- 'Type mismatch: can not convert 9.22337203685478e+18 to integer'
+- 'Type mismatch: can not convert 1.84467440737096e+19 to integer'
...
-- gh-3810: make sure that if space contains integers in range
-- [INT64_MAX, UINT64_MAX], they are handled inside SQL in a
diff --git a/test/sql/integer-overflow.test.lua b/test/sql/integer-overflow.test.lua
index 1b3e8ce..17051f9 100644
--- a/test/sql/integer-overflow.test.lua
+++ b/test/sql/integer-overflow.test.lua
@@ -27,12 +27,12 @@ box.execute('SELECT 18446744073709551616;')
box.execute('SELECT CAST(\'9223372036854775808\' AS INTEGER);')
box.execute('SELECT CAST(\'18446744073709551616\' AS INTEGER);')
-- Due to inexact represantation of large integers in terms of
--- floating point numbers, numerics with value < INT64_MAX
--- have INT64_MAX + 1 value in integer representation:
--- float 9223372036854775800 -> int (9223372036854775808),
--- with error due to conversion = 8.
+-- floating point numbers, numerics with value < UINT64_MAX
+-- have UINT64_MAX + 1 value in integer representation:
+-- float 18446744073709551600 -> int (18446744073709551616),
+-- with error due to conversion = 16.
--
-box.execute('SELECT CAST(9223372036854775807.0 AS INTEGER);')
+box.execute('SELECT CAST(18446744073709551600. AS INTEGER);')
-- gh-3810: make sure that if space contains integers in range
-- [INT64_MAX, UINT64_MAX], they are handled inside SQL in a
-- proper way, which now means that an error is raised.
More information about the Tarantool-patches
mailing list