From: "n.pettik" <korablev@tarantool.org> To: tarantool-patches@freelists.org Cc: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Subject: [tarantool-patches] Re: [PATCH 6/6] sql: allow to specify UNSIGNED column type Date: Wed, 17 Jul 2019 03:53:32 +0300 [thread overview] Message-ID: <552F96C1-DAC5-4F18-9F5A-BF50C6BBC205@tarantool.org> (raw) In-Reply-To: <a5a77dcf-9661-a5b8-ea0f-082ed30b5a19@tarantool.org> > ------------------------- > 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.
next prev parent reply other threads:[~2019-07-17 0:53 UTC|newest] Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-06-07 15:37 [tarantool-patches] [PATCH 0/6] Introduce UNSIGNED type in SQL Nikita Pettik 2019-06-07 15:37 ` [tarantool-patches] [PATCH 1/6] sql: refactor sql_atoi64() Nikita Pettik 2019-06-11 21:11 ` [tarantool-patches] " Vladislav Shpilevoy 2019-07-01 14:20 ` n.pettik 2019-07-01 21:53 ` Vladislav Shpilevoy 2019-07-05 16:32 ` n.pettik 2019-06-07 15:37 ` [tarantool-patches] [PATCH 2/6] sql: separate VDBE memory holding positive and negative ints Nikita Pettik 2019-06-11 21:11 ` [tarantool-patches] " Vladislav Shpilevoy 2019-07-01 14:21 ` n.pettik 2019-07-01 21:53 ` Vladislav Shpilevoy 2019-07-05 16:33 ` n.pettik 2019-06-07 15:37 ` [tarantool-patches] [PATCH 3/6] sql: refactor arithmetic operations to support unsigned ints Nikita Pettik 2019-06-11 21:11 ` [tarantool-patches] " Vladislav Shpilevoy 2019-07-01 14:21 ` n.pettik 2019-07-01 21:53 ` Vladislav Shpilevoy 2019-07-05 16:36 ` n.pettik 2019-07-10 22:49 ` Vladislav Shpilevoy 2019-07-17 12:24 ` n.pettik 2019-06-07 15:37 ` [tarantool-patches] [PATCH 4/6] sql: make built-in functions operate on unsigned values Nikita Pettik 2019-06-11 21:11 ` [tarantool-patches] " Vladislav Shpilevoy 2019-07-01 14:21 ` n.pettik 2019-07-01 21:53 ` Vladislav Shpilevoy 2019-07-05 16:36 ` n.pettik 2019-07-10 22:49 ` Vladislav Shpilevoy 2019-07-17 0:53 ` n.pettik 2019-06-07 15:37 ` [tarantool-patches] [PATCH 5/6] sql: introduce extended range for INTEGER type Nikita Pettik 2019-06-11 21:11 ` [tarantool-patches] " Vladislav Shpilevoy 2019-07-01 14:21 ` n.pettik 2019-07-01 21:53 ` Vladislav Shpilevoy 2019-07-24 15:59 ` Konstantin Osipov 2019-07-24 16:54 ` n.pettik 2019-07-24 17:09 ` Konstantin Osipov 2019-06-07 15:37 ` [tarantool-patches] [PATCH 6/6] sql: allow to specify UNSIGNED column type Nikita Pettik 2019-07-01 21:53 ` [tarantool-patches] " Vladislav Shpilevoy 2019-07-05 16:36 ` n.pettik 2019-07-10 22:49 ` Vladislav Shpilevoy 2019-07-11 21:25 ` Vladislav Shpilevoy 2019-07-17 0:53 ` n.pettik [this message] 2019-07-18 20:18 ` Vladislav Shpilevoy 2019-07-18 20:56 ` n.pettik 2019-07-18 21:08 ` Vladislav Shpilevoy 2019-07-18 21:13 ` Vladislav Shpilevoy 2019-07-22 10:20 ` n.pettik 2019-07-22 19:17 ` Vladislav Shpilevoy 2019-07-22 10:20 ` n.pettik 2019-07-17 0:54 ` n.pettik 2019-07-18 20:18 ` Vladislav Shpilevoy 2019-08-06 19:36 ` n.pettik 2019-07-24 13:01 ` [tarantool-patches] Re: [PATCH 0/6] Introduce UNSIGNED type in SQL Kirill Yukhin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=552F96C1-DAC5-4F18-9F5A-BF50C6BBC205@tarantool.org \ --to=korablev@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=v.shpilevoy@tarantool.org \ --subject='[tarantool-patches] Re: [PATCH 6/6] sql: allow to specify UNSIGNED column type' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox