[tarantool-patches] [PATCH v1 1/1] sql: remove unnecessary templates for bindings
Kirill Shcherbatov
kshcherbatov at tarantool.org
Wed May 16 20:14:03 MSK 2018
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
More information about the Tarantool-patches
mailing list