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 733B020BC3 for ; Tue, 19 Feb 2019 03:28:29 -0500 (EST) 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 psbWKND--ZkH for ; Tue, 19 Feb 2019 03:28:29 -0500 (EST) Received: from smtp40.i.mail.ru (smtp40.i.mail.ru [94.100.177.100]) (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 0195720AF1 for ; Tue, 19 Feb 2019 03:28:29 -0500 (EST) From: Nikita Pettik Subject: [tarantool-patches] [PATCH 2/3] sql: allow only string-like arguments for concatenation Date: Tue, 19 Feb 2019 11:28:22 +0300 Message-Id: In-Reply-To: References: In-Reply-To: References: 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 Cc: v.shpilevoy@tarantool.org, Nikita Pettik Original SQLite operator of concatenation accepts all types of arguments. If type of parameter is not TEXT, it is implicitly converted to TEXT (except for NULLs). That contradicts ANSI (it is regulated by [1]), so lets allow only TEXT and BLOB as argument type for concatenation. Moreover, they both must be of the same type at the same time (i.e. both TEXT or BLOB). [1] SQL ANSI 2013, 9.5 Result of data type combination Part of #3544 --- src/box/errcode.h | 1 + src/box/sql/vdbe.c | 54 +++++++++++++++++++++++++++++++++++++++-- test/box/misc.result | 1 + test/sql-tap/autoinc.test.lua | 12 ++++----- test/sql-tap/e_select1.test.lua | 4 +-- test/sql-tap/func.test.lua | 4 +-- test/sql-tap/sort.test.lua | 6 ++--- test/sql-tap/tkt2192.test.lua | 4 +-- test/sql-tap/trigger5.test.lua | 4 +-- test/sql/types.result | 32 ++++++++++++++++++++++++ test/sql/types.test.lua | 12 +++++++++ 11 files changed, 115 insertions(+), 19 deletions(-) diff --git a/src/box/errcode.h b/src/box/errcode.h index f7dbb948e..8ef60e861 100644 --- a/src/box/errcode.h +++ b/src/box/errcode.h @@ -227,6 +227,7 @@ struct errcode_record { /*172 */_(ER_ROWID_OVERFLOW, "Rowid is overflowed: too many entries in ephemeral space") \ /*173 */_(ER_DROP_COLLATION, "Can't drop collation %s : %s") \ /*174 */_(ER_ILLEGAL_COLLATION_MIX, "Illegal mix of collations") \ + /*175 */_(ER_INCONSISTENT_TYPES, "Inconsistent types: expected %s got %s") \ /* * !IMPORTANT! Please follow instructions at start of the file diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index c370b81ec..37955858d 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -619,6 +619,32 @@ vdbe_add_new_autoinc_id(struct Vdbe *vdbe, int64_t id) return 0; } +/** + * Simple type to str convertor. It is used to simplify + * error reporting. + */ +static char * +mem_type_to_str(const struct Mem *p) +{ + assert(p != NULL); + switch (p->flags & MEM_PURE_TYPE_MASK) { + case MEM_Null: + return "NULL"; + case MEM_Str: + return "TEXT"; + case MEM_Int: + return "INTEGER"; + case MEM_Real: + return "REAL"; + case MEM_Blob: + return "BLOB"; + case MEM_Bool: + return "BOOLEAN"; + default: + unreachable(); + } +} + /* * Execute as much of a VDBE program as we can. * This is the core of sql_step(). @@ -1515,6 +1541,9 @@ case OP_ResultRow: { * It is illegal for P1 and P3 to be the same register. Sometimes, * if P3 is the same register as P2, the implementation is able * to avoid a memcpy(). + * + * Concatenation operator accepts only arguments of string-like + * types (i.e. TEXT and BLOB). */ case OP_Concat: { /* same as TK_CONCAT, in1, in2, out3 */ i64 nByte; @@ -1527,9 +1556,30 @@ case OP_Concat: { /* same as TK_CONCAT, in1, in2, out3 */ sqlVdbeMemSetNull(pOut); break; } + /* + * Concatenation operation can be applied only to + * strings and blobs. + */ + uint32_t str_type_p1 = pIn1->flags & (MEM_Blob | MEM_Str); + uint32_t str_type_p2 = pIn2->flags & (MEM_Blob | MEM_Str); + if (str_type_p1 == 0 || str_type_p2 == 0) { + char *inconsistent_type = str_type_p1 == 0 ? + mem_type_to_str(pIn1) : + mem_type_to_str(pIn2); + diag_set(ClientError, ER_INCONSISTENT_TYPES, "TEXT or BLOB", + inconsistent_type); + rc = SQL_TARANTOOL_ERROR; + goto abort_due_to_error; + } + + /* Moreover, both operands must be of the same type. */ + if (str_type_p1 != str_type_p2) { + diag_set(ClientError, ER_INCONSISTENT_TYPES, + mem_type_to_str(pIn2), mem_type_to_str(pIn1)); + rc = SQL_TARANTOOL_ERROR; + goto abort_due_to_error; + } if (ExpandBlob(pIn1) || ExpandBlob(pIn2)) goto no_mem; - Stringify(pIn1); - Stringify(pIn2); nByte = pIn1->n + pIn2->n; if (nByte>db->aLimit[SQL_LIMIT_LENGTH]) { goto too_big; diff --git a/test/box/misc.result b/test/box/misc.result index 97189ecbb..345593655 100644 --- a/test/box/misc.result +++ b/test/box/misc.result @@ -502,6 +502,7 @@ t; 172: box.error.ROWID_OVERFLOW 173: box.error.DROP_COLLATION 174: box.error.ILLEGAL_COLLATION_MIX + 175: box.error.INCONSISTENT_TYPES ... test_run:cmd("setopt delimiter ''"); --- diff --git a/test/sql-tap/autoinc.test.lua b/test/sql-tap/autoinc.test.lua index 1110a2472..5ec9fd632 100755 --- a/test/sql-tap/autoinc.test.lua +++ b/test/sql-tap/autoinc.test.lua @@ -686,11 +686,11 @@ test:do_test( DROP TRIGGER t3928r2; CREATE TRIGGER t3928r3 BEFORE UPDATE ON t3928 WHEN new.b=='456' BEGIN - INSERT INTO t3928(b) VALUES('before-int-' || new.b); + INSERT INTO t3928(b) VALUES('before-int-' || CAST(new.b AS TEXT)); END; CREATE TRIGGER t3928r4 AFTER UPDATE ON t3928 WHEN new.b=='456' BEGIN - INSERT INTO t3928(b) VALUES('after-int-' || new.b); + INSERT INTO t3928(b) VALUES('after-int-' || CAST(new.b AS TEXT)); END; DELETE FROM t3928 WHERE a!=1; UPDATE t3928 SET b=456 WHERE a=1; @@ -725,12 +725,12 @@ test:do_test( DELETE FROM t3928; CREATE TABLE t3928c(y INTEGER PRIMARY KEY AUTOINCREMENT, z TEXT); CREATE TRIGGER t3928br1 BEFORE DELETE ON t3928b BEGIN - INSERT INTO t3928(b) VALUES('before-del-'||old.x); - INSERT INTO t3928c(z) VALUES('before-del-'||old.x); + INSERT INTO t3928(b) VALUES('before-del-'|| CAST(old.x AS TEXT)); + INSERT INTO t3928c(z) VALUES('before-del-'|| CAST(old.x AS TEXT)); END; CREATE TRIGGER t3928br2 AFTER DELETE ON t3928b BEGIN - INSERT INTO t3928(b) VALUES('after-del-'||old.x); - INSERT INTO t3928c(z) VALUES('after-del-'||old.x); + INSERT INTO t3928(b) VALUES('after-del-'|| CAST(old.x AS TEXT)); + INSERT INTO t3928c(z) VALUES('after-del-'|| CAST(old.x AS TEXT)); END; DELETE FROM t3928b; SELECT * FROM t3928 ORDER BY a; diff --git a/test/sql-tap/e_select1.test.lua b/test/sql-tap/e_select1.test.lua index 271fa1547..cdd088209 100755 --- a/test/sql-tap/e_select1.test.lua +++ b/test/sql-tap/e_select1.test.lua @@ -875,7 +875,7 @@ test:do_select_tests( 0, -22.18, -2.2, -23.18, "suiters", 1, 68, "", 67, "quartets", 0, -31.3, -1.04, -32.3, "aspen", 0, 1, 63, 0, "-26"}}, - {"3", "SELECT 32*32, d||e FROM z2", {1024, "", 1024, "36.06.0"}}, + {"3", "SELECT 32*32, CAST(d AS TEXT) || CAST(e AS TEXT) FROM z2", {1024, "", 1024, "36.06.0"}}, }) -- Test cases e_select-4.5.* and e_select-4.6.* together show that: @@ -1136,7 +1136,7 @@ test:do_select_tests( {"1.1", "SELECT up FROM c1 GROUP BY up HAVING count(*)>3", {"x"}}, {"1.2", "SELECT up FROM c1 GROUP BY up HAVING sum(down)>16", {"y"}}, {"1.3", "SELECT up FROM c1 GROUP BY up HAVING sum(down)<16", {"x"}}, - {"1.4", "SELECT up||down FROM c1 GROUP BY (down<5) HAVING max(down)<10", {"x4"}}, + {"1.4", "SELECT up|| CAST(down AS TEXT) FROM c1 GROUP BY (down<5) HAVING max(down)<10", {"x4"}}, {"2.1", "SELECT up FROM c1 GROUP BY up HAVING down>10", {"y"}}, {"2.2", "SELECT up FROM c1 GROUP BY up HAVING up='y'", {"y"}}, diff --git a/test/sql-tap/func.test.lua b/test/sql-tap/func.test.lua index e6b7956aa..495be7d29 100755 --- a/test/sql-tap/func.test.lua +++ b/test/sql-tap/func.test.lua @@ -475,7 +475,7 @@ test:do_catchsql_test( test:do_catchsql_test( "func-4.10", [[ - SELECT 'x' || round(c,a) || 'y' FROM t1 ORDER BY a + SELECT 'x' || CAST(round(c,a) AS TEXT) || 'y' FROM t1 ORDER BY a ]], { -- 0, {"x3.0y", "x-12345.68y", "x-5.0y"} @@ -863,7 +863,7 @@ test:do_test( test:do_execsql_test( "func-8.2", [[ - SELECT max('z+'||a||'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOP') FROM t2; + SELECT max('z+'|| CAST(a AS TEXT) ||'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOP') FROM t2; ]], { -- "z+67890abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOP" diff --git a/test/sql-tap/sort.test.lua b/test/sql-tap/sort.test.lua index df666070f..a84c549cc 100755 --- a/test/sql-tap/sort.test.lua +++ b/test/sql-tap/sort.test.lua @@ -238,7 +238,7 @@ test:do_execsql_test( test:do_execsql_test( "sort-2.1.1", [[ - UPDATE t1 SET v='x' || -flt; + UPDATE t1 SET v='x' || CAST(-flt AS TEXT); UPDATE t1 SET v='x-2b' where v=='x-0.123'; SELECT v FROM t1 ORDER BY v; ]], { @@ -338,7 +338,7 @@ test:do_execsql_test( test:do_execsql_test( "sort-4.2", [[ - SELECT n||'' FROM t1 ORDER BY 1; + SELECT CAST(n AS TEXT) || '' FROM t1 ORDER BY 1; ]], { -- "1", "10", "11", "12", "2", "3", "4", "5", "6", "7", "8", "9" @@ -358,7 +358,7 @@ test:do_execsql_test( test:do_execsql_test( "sort-4.4", [[ - SELECT n||'' FROM t1 ORDER BY 1 DESC; + SELECT CAST(n AS TEXT) || '' FROM t1 ORDER BY 1 DESC; ]], { -- "9", "8", "7", "6", "5", "4", "3", "2", "12", "11", "10", "1" diff --git a/test/sql-tap/tkt2192.test.lua b/test/sql-tap/tkt2192.test.lua index eb53863d4..b118d6310 100755 --- a/test/sql-tap/tkt2192.test.lua +++ b/test/sql-tap/tkt2192.test.lua @@ -146,7 +146,7 @@ test:do_execsql_test( test:do_execsql_test( "tkt2192-2.3", [[ - SELECT x.a || '/' || x.b || '/' || y.b + SELECT CAST(x.a AS TEXT) || '/' || CAST(x.b AS TEXT) || '/' || CAST(y.b AS TEXT) FROM v1 AS x JOIN v1 AS y ON x.a=y.a AND x.b