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 5199F24B4A for ; Wed, 16 May 2018 13:14: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 hvQClahWB8Pj for ; Wed, 16 May 2018 13:14:07 -0400 (EDT) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 DC6A524B28 for ; Wed, 16 May 2018 13:14:06 -0400 (EDT) From: Kirill Shcherbatov Subject: [tarantool-patches] [PATCH v1 1/1] sql: remove unnecessary templates for bindings Date: Wed, 16 May 2018 20:14:03 +0300 Message-Id: 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, Kirill Shcherbatov Removed ?N binding, changed $V to $N semantics to match other vendors standarts. Closes #2948 --- src/box/sql/expr.c | 13 +++++++----- test/sql-tap/e_expr.test.lua | 50 +++----------------------------------------- test/sql/iproto.result | 33 +++++++++++++++++++++++++---- test/sql/iproto.test.lua | 14 +++++++++++-- 4 files changed, 52 insertions(+), 58 deletions(-) diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 1b51823..cd2f549 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -1073,11 +1073,11 @@ sqlite3ExprFunction(Parse * pParse, ExprList * pList, Token * pToken) * Wildcards consisting of a single "?" are assigned the next sequential * variable number. * - * Wildcards of the form "?nnn" are assigned the number "nnn". We make + * Wildcards of the form "$nnn" are assigned the number "nnn". We make * sure "nnn" is not too big to avoid a denial of service attack when * the SQL statement comes from an external source. * - * Wildcards of the form ":aaa", "@aaa", or "$aaa" are assigned the same number + * Wildcards of the form ":aaa", "@aaa", are assigned the same number * as the previous instance of the same wildcard. Or if this is the first * instance of the wildcard, the next sequential variable number is * assigned. @@ -1104,7 +1104,10 @@ sqlite3ExprAssignVarNumber(Parse * pParse, Expr * pExpr, u32 n) } else { int doAdd = 0; if (z[0] == '?') { - /* Wildcard of the form "?nnn". Convert "nnn" to an integer and + sqlite3ErrorMsg(pParse, "Unsupported variable format"); + return; + } else if (z[0] == '$') { + /* Wildcard of the form "$nnn". Convert "nnn" to an integer and * use it as the variable number */ i64 i; @@ -1129,7 +1132,7 @@ sqlite3ExprAssignVarNumber(Parse * pParse, Expr * pExpr, u32 n) doAdd = 1; } } else { - /* Wildcards like ":aaa", "$aaa" or "@aaa". Reuse the same variable + /* Wildcards like ":aaa", or "@aaa". Reuse the same variable * number as the prior appearance of the same name, or if the name * has never appeared before, reuse the same variable number */ @@ -3828,7 +3831,7 @@ sqlite3ExprCodeTarget(Parse * pParse, Expr * pExpr, int target) const char *z = sqlite3VListNumToName(pParse->pVList, pExpr->iColumn); - assert(pExpr->u.zToken[0] == '?' + assert(pExpr->u.zToken[0] == '$' || strcmp(pExpr->u.zToken, z) == 0); pParse->pVList[0] = 0; /* Indicate VList may no longer be enlarged */ sqlite3VdbeAppendP4(v, (char *)z, P4_STATIC); diff --git a/test/sql-tap/e_expr.test.lua b/test/sql-tap/e_expr.test.lua index d0f6895..90cd15f 100755 --- a/test/sql-tap/e_expr.test.lua +++ b/test/sql-tap/e_expr.test.lua @@ -1,6 +1,6 @@ #!/usr/bin/env tarantool test = require("sqltester") -test:plan(14750) +test:plan(14749) --!./tcltestrunner.lua -- 2010 July 16 @@ -1188,48 +1188,6 @@ if (0>0) then X(492, "X!cmd", [=[["do_test",[["tn"],".res"],[["list","set","",["res"]]],["result"]]]=]) end - -- EVIDENCE-OF: R-33509-39458 A question mark followed by a number NNN - -- holds a spot for the NNN-th parameter. NNN must be between 1 and - -- SQLITE_MAX_VARIABLE_NUMBER. - -- - mvn = SQLITE_MAX_VARIABLE_NUMBER - parameter_test("e_expr-11.1", string.format([[ - SELECT ?1, ?123, ?%s, ?123, ?4 - ]], SQLITE_MAX_VARIABLE_NUMBER), string.format("1 ?1 123 ?123 %s ?%s 4 ?4", mvn, mvn), "-1 -123 -"..mvn.." -123 -4") - errmsg = "variable number must be between ?1 and ?"..SQLITE_MAX_VARIABLE_NUMBER.."" - for _ in X(0, "X!foreach", [=[["tn param_number",[["list","2","0","3",[["expr",[["SQLITE_MAX_VARIABLE_NUMBER"],"+1"]]],"4",[["expr",[["SQLITE_MAX_VARIABLE_NUMBER"],"+2"]]],"5","12345678903456789034567890234567890","6","2147483648","7","2147483649","8","4294967296","9","4294967297","10","9223372036854775808","11","9223372036854775809","12","18446744073709551616","13","18446744073709551617"]]]]=]) do - test:do_catchsql_test( - "e_expr-11.1."..tn, - "SELECT ?"..param_number.."", { - 1, errmsg - }) - - end - -- EVIDENCE-OF: R-33670-36097 A question mark that is not followed by a - -- number creates a parameter with a number one greater than the largest - -- parameter number already assigned. - -- - -- EVIDENCE-OF: R-42938-07030 If this means the parameter number is - -- greater than SQLITE_MAX_VARIABLE_NUMBER, it is an error. - -- - parameter_test("e_expr-11.2.1", "SELECT ?", "1 {}", -1) - parameter_test("e_expr-11.2.2", "SELECT ?, ?", "1 {} 2 {}", "-1 -2") - parameter_test("e_expr-11.2.3", "SELECT ?5, ?", "5 ?5 6 {}", "-5 -6") - parameter_test("e_expr-11.2.4", "SELECT ?, ?5", "1 {} 5 ?5", "-1 -5") - parameter_test("e_expr-11.2.5", "SELECT ?, ?456, ?", [[ - 1 {} 456 ?456 457 {} - ]], "-1 -456 -457") - parameter_test("e_expr-11.2.5", "SELECT ?, ?456, ?4, ?", [[ - 1 {} 456 ?456 4 ?4 457 {} - ]], "-1 -456 -4 -457") - for _ in X(0, "X!foreach", [=[["tn sql",[["list","1",["SELECT ?",["mvn"],", ?"],"2",["SELECT ?",[["expr",[["mvn"],"-5"]]],", ?, ?, ?, ?, ?, ?"],"3",["SELECT ?",[["expr",["mvn"]]],", ?5, ?6, ?"]]]]]=]) do - test:do_catchsql_test( - "e_expr-11.3."..tn, - sql, { - 1, "too many SQL variables" - }) - - end -- EVIDENCE-OF: R-11620-22743 A colon followed by an identifier name -- holds a spot for a named parameter with the name :AAAA. -- @@ -1287,7 +1245,6 @@ if (0>0) then -- error. -- parameter_test("e_expr-11.6.1", "SELECT ?, @abc", "1 {} 2 @abc", "-1 -2") - parameter_test("e_expr-11.6.2", "SELECT ?123, :a1", "123 ?123 124 :a1", "-123 -124") parameter_test("e_expr-11.6.3", "SELECT $a, ?8, ?, $b, ?2, $c", [[ 1 $a 8 ?8 9 {} 10 $b 2 ?2 11 $c ]], "-1 -8 -9 -10 -2 -11") @@ -1503,11 +1460,10 @@ local test_cases12 ={ {8, "CURRENT_TIMESTAMP"}, {9, "?"}, - {10, "?123"}, {11, "@hello"}, {12, ":world"}, - {13, "$tcl"}, - {14, "$tcl(array)"}, + {13, ":tcl"}, + {14, ":tcl(array)"}, {15, "cname"}, {16, "tblname.cname"}, diff --git a/test/sql/iproto.result b/test/sql/iproto.result index b251c80..4305eb9 100644 --- a/test/sql/iproto.result +++ b/test/sql/iproto.result @@ -164,7 +164,7 @@ parameters = {} parameters[1] = {} --- ... -parameters[1]['$value3'] = 1 +parameters[1][':value3'] = 1 --- ... parameters[2] = 2 @@ -188,10 +188,10 @@ parameters[6] = {} parameters[6]['@value2'] = 6 --- ... -cn:execute('select $value3, ?, :value1, ?, ?, @value2, ?, $value3', parameters) +cn:execute('select :value3, ?, :value1, ?, ?, @value2, ?, :value3', parameters) --- -- metadata: [{'name': $value3}, {'name': '?'}, {'name': ':value1'}, {'name': '?'}, - {'name': '?'}, {'name': '@value2'}, {'name': '?'}, {'name': '$value3'}] +- metadata: [{'name': ':value3'}, {'name': '?'}, {'name': ':value1'}, {'name': '?'}, + {'name': '?'}, {'name': '@value2'}, {'name': '?'}, {'name': ':value3'}] rows: - [1, 2, 3, 4, 5, 6, null, 1] ... @@ -422,6 +422,31 @@ cn:execute('drop table if exists test3') --- - rowcount: 0 ... +-- +-- gh-2948: sql: remove unnecessary templates for binding parameters +-- +cn:execute('select ?1, ?2, ?3', {1, 2, 3}) +--- +- error: 'Failed to execute SQL statement: Unsupported variable format' +... +parameters = {} +--- +... +parameters[1] = 11 +--- +... +parameters[2] = 22 +--- +... +parameters[3] = 33 +--- +... +cn:execute('select $2, $1, $3', parameters) +--- +- metadata: [{'name': $2}, {'name': '$1'}, {'name': '$3'}] + rows: + - [22, 11, 33] +... -- gh-2602 obuf_alloc breaks the tuple in different slabs _ = space:replace{1, 1, string.rep('a', 4 * 1024 * 1024)} --- diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua index 9d7c825..cbeaddd 100644 --- a/test/sql/iproto.test.lua +++ b/test/sql/iproto.test.lua @@ -60,7 +60,7 @@ cn:execute('select ?, :value1, @value2', parameters) parameters = {} parameters[1] = {} -parameters[1]['$value3'] = 1 +parameters[1][':value3'] = 1 parameters[2] = 2 parameters[3] = {} parameters[3][':value1'] = 3 @@ -68,7 +68,7 @@ parameters[4] = 4 parameters[5] = 5 parameters[6] = {} parameters[6]['@value2'] = 6 -cn:execute('select $value3, ?, :value1, ?, ?, @value2, ?, $value3', parameters) +cn:execute('select :value3, ?, :value1, ?, ?, @value2, ?, :value3', parameters) -- Try not-integer types. msgpack = require('msgpack') @@ -161,6 +161,16 @@ cn:execute('insert into test3 values (1, 1, 1), (2, 2, 2), (3, 3, 3)') cn:execute('drop table test3') cn:execute('drop table if exists test3') +-- +-- gh-2948: sql: remove unnecessary templates for binding parameters +-- +cn:execute('select ?1, ?2, ?3', {1, 2, 3}) +parameters = {} +parameters[1] = 11 +parameters[2] = 22 +parameters[3] = 33 +cn:execute('select $2, $1, $3', parameters) + -- gh-2602 obuf_alloc breaks the tuple in different slabs _ = space:replace{1, 1, string.rep('a', 4 * 1024 * 1024)} res = cn:execute('select * from test') -- 2.7.4