[tarantool-patches] Re: [PATCH 6/6] sql: allow to specify UNSIGNED column type
n.pettik
korablev at tarantool.org
Wed Jul 17 03:53:32 MSK 2019
> -------------------------
> vdbemem.c:661
>
>> 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);
>> pMem->u.r = pMem->u.i;
>> return 0;
>> }
>> return ! sqlAtoF(pMem->z, &pMem->u.r, pMem->n);
>> }
>
> Here you use assign 'pMem.u.i' to 'pMem.u.r' without checking
> its sign. In theory something like that should work wrong now:
>
> box.execute("SELECT CAST('9223372036854775837' AS REAL)")
>
> But you need to somehow write that number as blob.
Thanks, you are absolutely right:
tarantool> SELECT CAST(x'3138343436373434303733373039353531363135' AS real)
---
- metadata:
- name: CAST(x'3138343436373434303733373039353531363135' AS real)
type: number
rows:
- [-1]
...
Fix:
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index 64acb5d41..087cd20f1 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -658,7 +658,10 @@ sqlVdbeMemCast(Mem * pMem, enum field_type type)
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;
+ if (is_neg)
+ pMem->u.r = pMem->u.i;
+ else
+ pMem->u.r = pMem->u.u;
return 0;
}
return ! sqlAtoF(pMem->z, &pMem->u.r, pMem->n);
diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua
index 315f49d08..3027657a5 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(82)
+test:plan(84)
--!./tcltestrunner.lua
-- 2005 June 25
@@ -793,6 +793,18 @@ if true then --test:execsql("PRAGMA encoding")[1][1]=="UTF-8" then
})
end
+test:do_execsql_test(
+ "case-3.25",
+ [[
+ SELECT CAST(x'3138343436373434303733373039353531363135' AS REAL);
+ ]], { 1.844674407371e+19 } )
+
+test:do_execsql_test(
+ "case-3.26",
+ [[
+ SELECT CAST(x'3138343436373434303733373039353531363135' AS INT);
+ ]], { 18446744073709551615LL } )
+
test:do_execsql_test(
"case-3.31",
[[
> -------------------------
> vdbetrace.c:90
>
> Function sqlVdbeExpandSql stringifies integers, but
> misses a version for big unsigned.
diff --git a/src/box/sql/vdbetrace.c b/src/box/sql/vdbetrace.c
index f78d5b093..2ee9f668c 100644
--- a/src/box/sql/vdbetrace.c
+++ b/src/box/sql/vdbetrace.c
@@ -151,6 +151,8 @@ sqlVdbeExpandSql(Vdbe * p, /* The prepared statement being evaluated */
sqlStrAccumAppend(&out, "NULL", 4);
} else if (pVar->flags & MEM_Int) {
sqlXPrintf(&out, "%lld", pVar->u.i);
+ } else if (pVar->flags & MEM_UInt) {
+ sqlXPrintf(&out, "%llu", pVar->u.u);
} else if (pVar->flags & MEM_Real) {
sqlXPrintf(&out, "%!.15g", pVar->u.r);
} else if (pVar->flags & MEM_Str) {
> -------------------------
> vdbe.c:307
>
>> case FIELD_TYPE_INTEGER:
>> case FIELD_TYPE_UNSIGNED:
>> if ((record->flags & MEM_Int) == MEM_Int)
>> return 0;
>> 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);
>> return 0;
>> }
>
> It is a part of function mem_apply_type. When target type is
> UNSIGNED, and a value is MEM_Int, you do nothing. Why? Looks like
> it is possible to pass here a negative value, and CAST UNSIGNED
> would do nothing.
Basically, this function implements sort of implicit cast
which takes place before comparison/assignment.
For comparisons it makes no sense - we can compare
integer with unsigned value - the latter is always greater.
For assignment it is also meaningless: if we attempt
at inserting negative values to unsigned field appropriate
error will be raised anyway. If you can come up with
specific example, let’s discuss it.
More information about the Tarantool-patches
mailing list