From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 94A3B2BC9D for ; Thu, 27 Sep 2018 16:24:07 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id U1vYCVu5fIQY for ; Thu, 27 Sep 2018 16:24:07 -0400 (EDT) Received: from smtp56.i.mail.ru (smtp56.i.mail.ru [217.69.128.36]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 266542BC33 for ; Thu, 27 Sep 2018 16:24:07 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH 4/6] sql: enforce implicit type conversions References: From: Vladislav Shpilevoy Message-ID: <3ba8b43c-c6d5-ff34-1fd2-c302eae1a760@tarantool.org> Date: Thu, 27 Sep 2018 23:24:05 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org, Nikita Pettik Cc: Georgy Kirichenko See 14 comments below, my review fixes on the branch and at the end of the email. On 17/09/2018 23:32, Nikita Pettik wrote: > From: Georgy Kirichenko > > Most DBs (at least PostgreSQL, Oracle and DB2) allow to process > following queries: > > CREATE TABLE t1 (id INT PRIMARY KEY); > INSERT INTO t1 VALUES (1.123), ('2'); > > In this particular case, 1.123 should be simply truncated to 1, > and '2' - converted to literal number 2. > > After passing real type to Tarantool (instead of ), example > above would fail without conversions. Thus, lets add implicit > conversions inside VDBE to make this example be legal. > However, still some types conversions must be prohibited. For instance, > can't be converted to integer or floating point numerical, > and vice versa. As I see in the tests that it looks weird now: I can insert into 'int' a 'float' value, but can not compare them: box.sql.execute("SELECT bar, foo, 42, 'awesome' FROM foobar WHERE foo<2.001") --- -- error: Can't convert 2.001 to INTEGER ... Why? We should either forbid insertion and comparison, or allow both of them. > --- > src/box/sql/expr.c | 17 +-- > src/box/sql/insert.c | 14 ++- > src/box/sql/vdbe.c | 195 +++++++++++++++++++++++++---------- > src/box/sql/vdbeInt.h | 10 +- > src/box/sql/vdbeapi.c | 12 ++- > src/box/sql/vdbemem.c | 178 ++++++++++++++++++-------------- > test/sql-tap/analyze9.test.lua | 2 +- > test/sql-tap/autoinc.test.lua | 2 +- > test/sql-tap/badutf1.test.lua | 23 +---- > test/sql-tap/boundary1.test.lua | 19 +++- > test/sql-tap/boundary2.test.lua | 20 +++- > test/sql-tap/cast.test.lua | 25 ++++- > test/sql-tap/collation.test.lua | 7 +- > test/sql-tap/e_select1.test.lua | 4 +- > test/sql-tap/fkey2.test.lua | 2 +- > test/sql-tap/in3.test.lua | 5 +- > test/sql-tap/insert3.test.lua | 40 +++---- > test/sql-tap/like3.test.lua | 66 +----------- > test/sql-tap/misc1.test.lua | 13 +-- > test/sql-tap/numcast.test.lua | 7 +- > test/sql-tap/quote.test.lua | 12 +-- > test/sql-tap/selectA.test.lua | 20 ++-- > test/sql-tap/sort.test.lua | 22 +--- > test/sql-tap/tkt-a8a0d2996a.test.lua | 103 +----------------- > test/sql-tap/types2.test.lua | 4 +- > test/sql/on-conflict.test.lua | 4 + > test/sql/persistency.result | 5 +- > test/sql/transition.result | 3 +- > test/sql/triggers.result | 12 +-- > test/sql/triggers.test.lua | 12 +-- > 30 files changed, 404 insertions(+), 454 deletions(-) > diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c > index 881666db3..b1d48ff72 100644 > --- a/src/box/sql/vdbe.c > +++ b/src/box/sql/vdbe.c > @@ -1945,11 +1970,27 @@ case OP_Cast: { /* in1 */ > testcase( pOp->p2==AFFINITY_INTEGER); > testcase( pOp->p2==AFFINITY_REAL); > pIn1 = &aMem[pOp->p1]; > - memAboutToChange(p, pIn1); > rc = ExpandBlob(pIn1); > - sqlite3VdbeMemCast(pIn1, pOp->p2); > + if (rc) > + goto abort_due_to_error; > + rc = sqlite3VdbeMemCast(pIn1, pOp->p2); > UPDATE_MAX_BLOBSIZE(pIn1); > - if (rc) goto abort_due_to_error; > + if (rc) { > + const char *format; > + if (pOp->p2 == AFFINITY_TEXT) > + format = "Can't convert %s to TEXT"; > + else if (pOp->p2 == AFFINITY_BLOB) > + format = "Can't convert %s to BLOB"; > + else if (pOp->p2 == AFFINITY_NUMERIC) > + format = "Can't convert %s to NUMERIC"; > + else if (pOp->p2 == AFFINITY_INTEGER) > + format = "Can't convert %s to INTEGER"; > + else if (pOp->p2 == AFFINITY_REAL) > + format = "Can't convert %s to REAL"; 1. Either we report uppercase affinity in all error messages, or in no one. I fixed to lowercase. 2. The same about format. Here you report first value and second type. In other places vice versa. I think, we should create a separate error code for this and use diag_set here and in other places. > + > + sqlite3VdbeError(p, format, sqlite3_value_text(pIn1)); > + goto abort_due_to_error; > + } > break; > } > #endif /* SQLITE_OMIT_CAST */ > @@ -2451,11 +2516,13 @@ case OP_IfNot: { /* jump, in1 */ > if (pIn1->flags & MEM_Null) { > c = pOp->p3; > } else { > -#ifdef SQLITE_OMIT_FLOATING_POINT > - c = sqlite3VdbeIntValue(pIn1)!=0; > -#else > - c = sqlite3VdbeRealValue(pIn1)!=0.0; > -#endif > + double v; > + if ((rc = sqlite3VdbeRealValue(pIn1, &v))) { > + sqlite3VdbeError(p, "Can't convert to numeric %s", 3. Why numeric? Maybe real? > + sqlite3_value_text(pIn1)); > + goto abort_due_to_error; > + } > + c = v != 0; > if (pOp->opcode==OP_IfNot) c = !c; > } > VdbeBranchTaken(c!=0, 2); > diff --git a/test/sql-tap/boundary1.test.lua b/test/sql-tap/boundary1.test.lua > index e35e1edbd..d149008b1 100755 > --- a/test/sql-tap/boundary1.test.lua > +++ b/test/sql-tap/boundary1.test.lua > @@ -7460,6 +7460,7 @@ test:do_execsql_test( > "SELECT a FROM t1 WHERE rowid > 9.22337303685477580800e+18 ORDER BY a DESC", > {}) > > +if false then 4. Why the test is commented? If it does not pass, then uncomment and check an error message. I did it just to check what is a problem and found "Can't convert 9.22337303685478e+18 to numeric". Why I can not convert? - it looks like a valid number. The same below for each 'if false', including boundary2.test.lua. > test:do_execsql_test( > "boundary1-2.65.gt.3", > "SELECT a FROM t1 WHERE rowid > 9.22337303685477580800e+18 ORDER BY rowid", > @@ -7485,6 +7487,7 @@ test:do_execsql_test( > "SELECT a FROM t1 WHERE rowid >= 9.22337303685477580800e+18 ORDER BY a DESC", > {}) > > +if false then > test:do_execsql_test( > "boundary1-2.65.ge.3", > "SELECT a FROM t1 WHERE rowid >= 9.22337303685477580800e+18 ORDER BY rowid", > @@ -7510,6 +7514,7 @@ test:do_execsql_test( > "SELECT a FROM t1 WHERE rowid < 9.22337303685477580800e+18 ORDER BY a DESC", > {64, 63, 62, 61, 60, 59, 58, 57, 56, 55, 54, 53, 52, 51, 50, 49, 48, 47, 46, 45, 44, 43, 42, 41, 40, 39, 38, 37, 36, 35, 34, 33, 32, 31, 30, 29, 28, 27, 26, 25, 24, 23, 22, 21, 20, 19, 18, 17, 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1}) > > +if false then > test:do_execsql_test( > "boundary1-2.65.lt.3", > "SELECT a FROM t1 WHERE rowid < 9.22337303685477580800e+18 ORDER BY rowid", > @@ -7535,6 +7541,7 @@ test:do_execsql_test( > "SELECT a FROM t1 WHERE rowid <= 9.22337303685477580800e+18 ORDER BY a DESC", > {64, 63, 62, 61, 60, 59, 58, 57, 56, 55, 54, 53, 52, 51, 50, 49, 48, 47, 46, 45, 44, 43, 42, 41, 40, 39, 38, 37, 36, 35, 34, 33, 32, 31, 30, 29, 28, 27, 26, 25, 24, 23, 22, 21, 20, 19, 18, 17, 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1}) > > +if false then > test:do_execsql_test( > "boundary1-2.65.le.3", > "SELECT a FROM t1 WHERE rowid <= 9.22337303685477580800e+18 ORDER BY rowid", > "SELECT a FROM t1 WHERE rowid > -9.22337303685477580800e+18 ORDER BY a DESC", > {64, 63, 62, 61, 60, 59, 58, 57, 56, 55, 54, 53, 52, 51, 50, 49, 48, 47, 46, 45, 44, 43, 42, 41, 40, 39, 38, 37, 36, 35, 34, 33, 32, 31, 30, 29, 28, 27, 26, 25, 24, 23, 22, 21, 20, 19, 18, 17, 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1}) > > +if false then > test:do_execsql_test( > "boundary1-2.66.gt.3", > "SELECT a FROM t1 WHERE rowid > -9.22337303685477580800e+18 ORDER BY rowid", > @@ -7585,6 +7595,7 @@ test:do_execsql_test( > "SELECT a FROM t1 WHERE rowid >= -9.22337303685477580800e+18 ORDER BY a DESC", > {64, 63, 62, 61, 60, 59, 58, 57, 56, 55, 54, 53, 52, 51, 50, 49, 48, 47, 46, 45, 44, 43, 42, 41, 40, 39, 38, 37, 36, 35, 34, 33, 32, 31, 30, 29, 28, 27, 26, 25, 24, 23, 22, 21, 20, 19, 18, 17, 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1}) > > +if false then > test:do_execsql_test( > "boundary1-2.66.ge.3", > "SELECT a FROM t1 WHERE rowid >= -9.22337303685477580800e+18 ORDER BY rowid", > @@ -7610,6 +7622,7 @@ test:do_execsql_test( > "SELECT a FROM t1 WHERE rowid < -9.22337303685477580800e+18 ORDER BY a DESC", > {}) > > +if false then > test:do_execsql_test( > "boundary1-2.66.lt.3", > "SELECT a FROM t1 WHERE rowid < -9.22337303685477580800e+18 ORDER BY rowid", > @@ -7635,6 +7649,7 @@ test:do_execsql_test( > "SELECT a FROM t1 WHERE rowid <= -9.22337303685477580800e+18 ORDER BY a DESC", > {}) > > +if false then > test:do_execsql_test( > "boundary1-2.66.le.3", > "SELECT a FROM t1 WHERE rowid <= -9.22337303685477580800e+18 ORDER BY rowid", > @@ -7644,10 +7659,10 @@ test:do_execsql_test( > test:do_execsql_test( > "boundary1-2.66.le.5", > "SELECT a FROM t1 WHERE rowid <= -9.22337303685477580800e+18 ORDER BY x", > {}) > - 5. Stray diff. > test:finish_test()> diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua > index e3b7b1248..9f810bfc1 100755 > --- a/test/sql-tap/cast.test.lua > +++ b/test/sql-tap/cast.test.lua > @@ -64,6 +64,7 @@ test:do_execsql_test( > -- > }) > > +if false then 6. For this and all previous and next 'if false' - the test is either needed or not. If a test makes no sense anymore - delete it. If it tests something real - fix the test or code. I do not understand why this test should not be run. It throws and error and here it is ok. You should check that the error is correct, an error message matches an expected one. > test:do_execsql_test( > "cast-1.5", > [[ > @@ -405,7 +411,9 @@ test:do_execsql_test( > "blob" > -- > }) > +end > > +if false then > test:do_execsql_test( > "cast-1.39", > [[ 7. Below I see this: "SELECT CAST(123.456 AS integer)". Why is it commented? This test should pass just like before the patch. Now it looks the same as throw an error in C on " (int) 123.456 ". It is explicit cast of a compatible type - why is it incorrect? > @@ -425,6 +433,7 @@ test:do_execsql_test( > "integer" > -- > }) > +end > > test:do_execsql_test( > "cast-1.41", I will not review other 'if false' comments. Please, check them all by yourself and either uncomment, or delete, or fix. > diff --git a/test/sql-tap/collation.test.lua b/test/sql-tap/collation.test.lua > index eb4f43a90..e8f09fdae 100755 > --- a/test/sql-tap/collation.test.lua > +++ b/test/sql-tap/collation.test.lua > @@ -249,9 +249,4 @@ local like_testcases = > > test:do_catchsql_set_test(like_testcases, prefix) > > -test:do_catchsql_test( > - "collation-2.5.0", > - 'CREATE TABLE test3 (a int, b int, c int, PRIMARY KEY (a, a COLLATE foo, b, c))', > - {1, "Collation 'FOO' does not exist"}) 8. It is not a test on types. It is a test on not existing collation. Please, fix the 'a' column type so as to fix the test. > - > test:finish_test() > diff --git a/test/sql-tap/e_select1.test.lua b/test/sql-tap/e_select1.test.lua > index b1e113cc9..109223611 100755 > --- a/test/sql-tap/e_select1.test.lua > +++ b/test/sql-tap/e_select1.test.lua > @@ -332,7 +332,7 @@ test:do_execsql_test( > > -- x1: 3 rows, 2 columns > INSERT INTO x1 VALUES(1,'24', 'converging'); > - INSERT INTO x1 VALUES(2, NULL, CAST(X'CB71' as TEXT)); > + INSERT INTO x1 VALUES(2, NULL, 'Pq');\ 9. Why? Explicit cast to text should work. > INSERT INTO x1 VALUES(3,'blonds', 'proprietary'); > > -- x2: 2 rows, 3 columns> diff --git a/test/sql-tap/like3.test.lua b/test/sql-tap/like3.test.lua > index ea6824ba7..7b42717ee 100755 > --- a/test/sql-tap/like3.test.lua > +++ b/test/sql-tap/like3.test.lua > @@ -67,72 +67,10 @@ test:do_execsql_test( > -- > }) > > -test:do_execsql_test( > - "like3-2.0", > - [[ > - CREATE TABLE t2(a INT PRIMARY KEY, b TEXT); > - INSERT INTO t2 SELECT a, b FROM t1; > - CREATE INDEX t2ba ON t2(b,a); > - SELECT a, b FROM t2 WHERE b GLOB 'ab*' ORDER BY +a; > - ]], { > - -- > - 1, "abc", 4, "abc" > - -- > - }) 10. Why? t2 consists of (int, text), t1 also does. So I must be able to do "INSERT INTO t2 SELECT a, b FROM t1", it is not? The same about other removed cases. > - > -test:do_execsql_test( > - "like3-2.1", > - [[ > - SELECT a, b FROM t2 WHERE +b GLOB 'ab*' ORDER BY +a; > - ]], { > - -- > - 1, "abc", 4, "abc" > - -- > - }) > - > -test:do_execsql_test( > - "like3-2.2", > - [[ > - SELECT a, b FROM t2 WHERE b>=x'6162' AND b GLOB 'ab*' > - ]], { > - -- > - 4, "abc" > - -- > - }) > - > -test:do_execsql_test( > - "like3-2.3", > - [[ > - SELECT a, b FROM t2 WHERE +b>=x'6162' AND +b GLOB 'ab*' > - ]], { > - -- > - 4, "abc" > - -- > - }) > - > -test:do_execsql_test( > - "like3-2.4", > - [[ > - SELECT a, b FROM t2 WHERE b GLOB 'ab*' AND b>=x'6162' > - ]], { > - -- > - 4, "abc" > - -- > - }) > - > -test:do_execsql_test( > - "like3-2.5", > - [[ > - SELECT a, b FROM t2 WHERE +b GLOB 'ab*' AND +b>=x'6162' > - ]], { > - -- > - 4, "abc" > - -- > - }) > test:execsql([[ > CREATE TABLE t3(x TEXT PRIMARY KEY COLLATE "unicode_ci"); > INSERT INTO t3(x) VALUES('aaa'),('abc'),('abd'),('abe'),('acz'); > - INSERT INTO t3(x) SELECT CAST(x AS blob) FROM t3; > +-- INSERT INTO t3(x) SELECT CAST(x AS blob) FROM t3; 11. Why I can not cast string to blob and vice versa? > ]]) > > diff --git a/test/sql-tap/selectA.test.lua b/test/sql-tap/selectA.test.lua > index 9161cba91..6540bf166 100755 > --- a/test/sql-tap/selectA.test.lua > +++ b/test/sql-tap/selectA.test.lua 12. I do not understand changes in this file. Neither why is it disabled and modified at the same time. It is not too big - 2536 lines. Please, make it work. > diff --git a/test/sql/on-conflict.test.lua b/test/sql/on-conflict.test.lua > index b2d8e0589..5ecd07e87 100644 > --- a/test/sql/on-conflict.test.lua > +++ b/test/sql/on-conflict.test.lua > @@ -1,5 +1,9 @@ > test_run = require('test_run').new() > +--- > +... > engine = test_run:get_cfg('engine') > +--- > +... 13. wtf ??? > box.sql.execute('pragma sql_default_engine=\''..engine..'\'') > -- > -- Check that original SQLite ON CONFLICT clause is really 14. I do not see tests for issues 3018, 3104, 2494 fixed in the previous commit. My review fixes: ======================================================================== commit cf7edb9d2c3fb4aa0c2b93dd8973bdfff5d6d6e3 Author: Vladislav Shpilevoy Date: Thu Sep 27 19:45:17 2018 +0300 Review fixes diff --git a/src/box/field_def.c b/src/box/field_def.c index 8dbead63f..3a9ff3703 100644 --- a/src/box/field_def.c +++ b/src/box/field_def.c @@ -33,6 +33,24 @@ #include "trivia/util.h" #include "key_def.h" +static const char *affinity_type_strs[] = { + /* [UNDEFINED] */ "undefined", + /* [BLOB - 'A'] */ "blob", + /* [TEXT - 'A'] */ "text", + /* [NUMERIC - 'A'] */ "numeric", + /* [INTEGER - 'A'] */ "integer", + /* [REAL - 'A'] */ "real", +}; + +const char * +affinity_type_str(enum affinity_type type) +{ + if (type < 'A') + return affinity_type_strs[type]; + else + return affinity_type_strs[type - 'A' + 1]; +} + const char *field_type_strs[] = { /* [FIELD_TYPE_ANY] = */ "any", /* [FIELD_TYPE_UNSIGNED] = */ "unsigned", diff --git a/src/box/field_def.h b/src/box/field_def.h index 05f80d409..de0ecec51 100644 --- a/src/box/field_def.h +++ b/src/box/field_def.h @@ -78,6 +78,11 @@ enum affinity_type { AFFINITY_INTEGER = 'D', AFFINITY_REAL = 'E', }; + +/** String name of @a type. */ +const char * +affinity_type_str(enum affinity_type type); + /** \endcond public */ extern const char *field_type_strs[]; diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index b1d48ff72..6f5da97f1 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -326,24 +326,24 @@ applyNumericAffinity(Mem *pRec, int bTryForInt) * @param affinity The affinity to be applied. */ static int -mem_apply_affinity(struct Mem *record, char affinity) +mem_apply_affinity(struct Mem *record, enum affinity_type affinity) { if ((record->flags & MEM_Null) != 0) return SQLITE_OK; - - if (affinity == AFFINITY_INTEGER) { + switch (affinity) { + case AFFINITY_INTEGER: if ((record->flags & MEM_Int) == MEM_Int) return 0; return sqlite3VdbeMemIntegerify(record, false); - } else if (affinity == AFFINITY_REAL) { + case AFFINITY_REAL: if ((record->flags & MEM_Real) == MEM_Real) return 0; return sqlite3VdbeMemRealify(record); - } else if (affinity == AFFINITY_NUMERIC) { + case AFFINITY_NUMERIC: if ((record->flags & (MEM_Real | MEM_Int)) != 0) return 0; return sqlite3VdbeMemNumerify(record); - } else if (affinity == AFFINITY_TEXT) { + case AFFINITY_TEXT: /* * Only attempt the conversion to TEXT if there is * an integer or real representation (BLOB and @@ -355,12 +355,13 @@ mem_apply_affinity(struct Mem *record, char affinity) } record->flags &= ~(MEM_Real | MEM_Int); return 0; - } else if (affinity == AFFINITY_BLOB) { + case AFFINITY_BLOB: if (record->flags & (MEM_Str | MEM_Blob)) record->flags |= MEM_Blob; return 0; + default: + return -1; } - return -1; } /* @@ -1971,27 +1972,15 @@ case OP_Cast: { /* in1 */ testcase( pOp->p2==AFFINITY_REAL); pIn1 = &aMem[pOp->p1]; rc = ExpandBlob(pIn1); - if (rc) + if (rc != 0) goto abort_due_to_error; rc = sqlite3VdbeMemCast(pIn1, pOp->p2); UPDATE_MAX_BLOBSIZE(pIn1); - if (rc) { - const char *format; - if (pOp->p2 == AFFINITY_TEXT) - format = "Can't convert %s to TEXT"; - else if (pOp->p2 == AFFINITY_BLOB) - format = "Can't convert %s to BLOB"; - else if (pOp->p2 == AFFINITY_NUMERIC) - format = "Can't convert %s to NUMERIC"; - else if (pOp->p2 == AFFINITY_INTEGER) - format = "Can't convert %s to INTEGER"; - else if (pOp->p2 == AFFINITY_REAL) - format = "Can't convert %s to REAL"; - - sqlite3VdbeError(p, format, sqlite3_value_text(pIn1)); - goto abort_due_to_error; - } - break; + if (rc == 0) + break; + sqlite3VdbeError(p, "Can't convert %s to %s", sqlite3_value_text(pIn1), + affinity_type_str(pOp->p2)); + goto abort_due_to_error; } #endif /* SQLITE_OMIT_CAST */ @@ -2394,7 +2383,7 @@ case OP_Or: { /* same as TK_OR, in1, in2, out3 */ if (pIn1->flags & MEM_Null) { v1 = 2; } else { - i64 i; + int64_t i; if ((rc = sqlite3VdbeIntValue(pIn1, &i)) != SQLITE_OK) { sqlite3VdbeError(p, "Can't convert to integer %s", sqlite3_value_text(pIn1)); @@ -2406,7 +2395,7 @@ case OP_Or: { /* same as TK_OR, in1, in2, out3 */ if (pIn2->flags & MEM_Null) { v2 = 2; } else { - i64 i; + int64_t i; if ((rc = sqlite3VdbeIntValue(pIn2, &i)) != SQLITE_OK) { sqlite3VdbeError(p, "Can't convert to integer %s", sqlite3_value_text(pIn2)); @@ -2443,7 +2432,7 @@ case OP_Not: { /* same as TK_NOT, in1, out2 */ pOut = &aMem[pOp->p2]; sqlite3VdbeMemSetNull(pOut); if ((pIn1->flags & MEM_Null)==0) { - i64 i; + int64_t i; if ((rc = sqlite3VdbeIntValue(pIn1, &i)) != SQLITE_OK) { sqlite3VdbeError(p, "Can't convert to integer %s", sqlite3_value_text(pIn1)); @@ -2467,7 +2456,7 @@ case OP_BitNot: { /* same as TK_BITNOT, in1, out2 */ pOut = &aMem[pOp->p2]; sqlite3VdbeMemSetNull(pOut); if ((pIn1->flags & MEM_Null)==0) { - i64 i; + int64_t i; if ((rc = sqlite3VdbeIntValue(pIn1, &i)) != SQLITE_OK) { sqlite3VdbeError(p, "Can't convert to integer %s", sqlite3_value_text(pIn1)); @@ -2780,19 +2769,9 @@ case OP_Affinity: { assert(pIn1 <= &p->aMem[(p->nMem+1 - p->nCursor)]); assert(memIsValid(pIn1)); if ((rc = mem_apply_affinity(pIn1, cAff)) != SQLITE_OK) { - const char *format; - if (cAff == AFFINITY_TEXT) - format = "Can't convert %s to TEXT"; - else if (cAff == AFFINITY_BLOB) - format = "Can't convert %s to BLOB"; - else if (cAff == AFFINITY_NUMERIC) - format = "Can't convert %s to NUMERIC"; - else if (cAff == AFFINITY_INTEGER) - format = "Can't convert %s to INTEGER"; - else if (cAff == AFFINITY_REAL) - format = "Can't convert %s to REAL"; - - sqlite3VdbeError(p, format, sqlite3_value_text(pIn1)); + sqlite3VdbeError(p, "Can't convert %s to %s", + sqlite3_value_text(pIn1), + affinity_type_str(cAff)); goto abort_due_to_error; } pIn1++; @@ -3501,7 +3480,7 @@ case OP_SeekGT: { /* jump, in3 */ if ((pIn3->flags & (MEM_Int|MEM_Real|MEM_Str))==MEM_Str) { applyNumericAffinity(pIn3, 0); } - i64 i; + int64_t i; if ((rc = sqlite3VdbeIntValue(pIn3, &i)) != SQLITE_OK) { sqlite3VdbeError(p, "Can't convert to integer %s", sqlite3_value_text(pIn1)); goto abort_due_to_error; diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c index ead527c27..04e60a079 100644 --- a/src/box/sql/vdbeapi.c +++ b/src/box/sql/vdbeapi.c @@ -214,7 +214,7 @@ sqlite3_value_double(sqlite3_value * pVal) int sqlite3_value_int(sqlite3_value * pVal) { - i64 i; + int64_t i; sqlite3VdbeIntValue((Mem *) pVal, &i); return (int)i; } @@ -222,7 +222,7 @@ sqlite3_value_int(sqlite3_value * pVal) sqlite_int64 sqlite3_value_int64(sqlite3_value * pVal) { - i64 i; + int64_t i; sqlite3VdbeIntValue((Mem *) pVal, &i); return i; } diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c index b8ceb11a7..f512ca47b 100644 --- a/src/box/sql/vdbemem.c +++ b/src/box/sql/vdbemem.c @@ -418,7 +418,7 @@ sqlite3VdbeMemRelease(Mem * p) * return the closest available 64-bit signed integer. */ static int -doubleToInt64(double r, i64 *i) +doubleToInt64(double r, int64_t *i) { #ifdef SQLITE_OMIT_FLOATING_POINT /* When floating-point is omitted, double and int64 are the same thing */ @@ -432,8 +432,8 @@ doubleToInt64(double r, i64 *i) * So we define our own static constants here using nothing * larger than a 32-bit integer constant. */ - static const i64 maxInt = LARGEST_INT64; - static const i64 minInt = SMALLEST_INT64; + static const int64_t maxInt = LARGEST_INT64; + static const int64_t minInt = SMALLEST_INT64; if (r <= (double)minInt) { *i = minInt; @@ -442,7 +442,7 @@ doubleToInt64(double r, i64 *i) *i = maxInt; return -1; } else { - *i = (i64) r; + *i = (int64_t) r; return *i != r; } #endif @@ -460,7 +460,7 @@ doubleToInt64(double r, i64 *i) * If pMem represents a string value, its encoding might be changed. */ int -sqlite3VdbeIntValue(Mem * pMem, i64 *i) +sqlite3VdbeIntValue(Mem * pMem, int64_t *i) { int flags; assert(EIGHT_BYTE_ALIGNMENT(pMem)); @@ -528,7 +528,7 @@ sqlite3VdbeMemIntegerify(Mem * pMem, bool is_forced) { assert(EIGHT_BYTE_ALIGNMENT(pMem)); - i64 i; + int64_t i; if (sqlite3VdbeIntValue(pMem, &i) == 0) { pMem->u.i = i; MemSetTypeFlag(pMem, MEM_Int); @@ -540,10 +540,10 @@ sqlite3VdbeMemIntegerify(Mem * pMem, bool is_forced) } double d; - if (sqlite3VdbeRealValue(pMem, &d) || (i64)d != d) { + if (sqlite3VdbeRealValue(pMem, &d) || (int64_t) d != d) { return SQLITE_ERROR; } - pMem->u.i = (i64)d; + pMem->u.i = (int64_t) d; MemSetTypeFlag(pMem, MEM_Int); return SQLITE_OK; } @@ -610,7 +610,7 @@ sqlite3VdbeMemCast(Mem * pMem, u8 aff) aff != AFFINITY_TEXT) return SQLITE_ERROR; switch (aff) { - case AFFINITY_BLOB:{ + case AFFINITY_BLOB: if (pMem->flags & MEM_Blob) return SQLITE_OK; if (pMem->flags & MEM_Str) { @@ -620,27 +620,21 @@ sqlite3VdbeMemCast(Mem * pMem, u8 aff) if (pMem->flags & MEM_Int || pMem->flags & MEM_Real) return sqlite3VdbeMemStringify(pMem, 0); return SQLITE_ERROR; - } - case AFFINITY_NUMERIC:{ + case AFFINITY_NUMERIC: return sqlite3VdbeMemNumerify(pMem); - } - case AFFINITY_INTEGER:{ + case AFFINITY_INTEGER: return sqlite3VdbeMemIntegerify(pMem, true); - } - case AFFINITY_REAL:{ + case AFFINITY_REAL: return sqlite3VdbeMemRealify(pMem); - } - default:{ + default: assert(aff == AFFINITY_TEXT); assert(MEM_Str == (MEM_Blob >> 3)); pMem->flags |= (pMem->flags & MEM_Blob) >> 3; sqlite3ValueApplyAffinity(pMem, AFFINITY_TEXT); assert(pMem->flags & MEM_Str || pMem->db->mallocFailed); - pMem->flags &= - ~(MEM_Int | MEM_Real | MEM_Blob | MEM_Zero); - } + pMem->flags &= ~(MEM_Int | MEM_Real | MEM_Blob | MEM_Zero); + return SQLITE_OK; } - return SQLITE_OK; } /* @@ -1326,7 +1320,7 @@ valueFromExpr(sqlite3 * db, /* The database connection */ sqlite3ValueFromExpr(db, pExpr->pLeft, affinity, &pVal) && pVal != 0) { if ((rc = sqlite3VdbeMemNumerify(pVal)) != SQLITE_OK) - goto exit; + return rc; if (pVal->flags & MEM_Real) { pVal->u.r = -pVal->u.r; } else if (pVal->u.i == SMALLEST_INT64) { @@ -1342,7 +1336,7 @@ valueFromExpr(sqlite3 * db, /* The database connection */ if (pVal == 0) goto no_mem; if ((rc = sqlite3VdbeMemNumerify(pVal)) != SQLITE_OK) - goto exit; + return rc; } #ifndef SQLITE_OMIT_BLOB_LITERAL else if (op == TK_BLOB) { @@ -1365,7 +1359,6 @@ valueFromExpr(sqlite3 * db, /* The database connection */ } *ppVal = pVal; - exit: return rc; no_mem: diff --git a/test/sql-tap/autoinc.test.lua b/test/sql-tap/autoinc.test.lua index 4157a61e4..3ec4af9cf 100755 --- a/test/sql-tap/autoinc.test.lua +++ b/test/sql-tap/autoinc.test.lua @@ -618,7 +618,7 @@ test:do_catchsql_test( INSERT INTO t2 VALUES('asd'); ]], { -- - 1, "Can't convert asd to INTEGER" + 1, "Can't convert asd to integer" -- }) diff --git a/test/sql/persistency.result b/test/sql/persistency.result index ed0526b43..05acdd0ae 100644 --- a/test/sql/persistency.result +++ b/test/sql/persistency.result @@ -67,7 +67,7 @@ box.sql.execute("SELECT bar, foo, 42, 'awesome' FROM foobar WHERE foo<2") ... box.sql.execute("SELECT bar, foo, 42, 'awesome' FROM foobar WHERE foo<2.001") --- -- error: Can't convert 2.001 to INTEGER +- error: Can't convert 2.001 to integer ... box.sql.execute("SELECT bar, foo, 42, 'awesome' FROM foobar WHERE foo<=2") --- @@ -179,7 +179,7 @@ box.sql.execute("SELECT \"name\", \"opts\" FROM \"_trigger\""); -- ... functional box.sql.execute("INSERT INTO foobar VALUES ('foobar trigger test', 8888)") --- -- error: Can't convert foobar trigger test to INTEGER +- error: Can't convert foobar trigger test to integer ... box.sql.execute("SELECT * FROM barfoo WHERE foo = 9999"); --- diff --git a/test/sql/transition.result b/test/sql/transition.result index 35869c69a..be2019139 100644 --- a/test/sql/transition.result +++ b/test/sql/transition.result @@ -64,7 +64,7 @@ box.sql.execute("SELECT bar, foo, 42, 'awesome' FROM foobar WHERE foo<2") ... box.sql.execute("SELECT bar, foo, 42, 'awesome' FROM foobar WHERE foo<2.001") --- -- error: Can't convert 2.001 to INTEGER +- error: Can't convert 2.001 to integer ... box.sql.execute("SELECT bar, foo, 42, 'awesome' FROM foobar WHERE foo<=2") ---