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 E22802BA33 for ; Sat, 30 Mar 2019 08:01:40 -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 64m1DTEbdtR0 for ; Sat, 30 Mar 2019 08:01:40 -0400 (EDT) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 0C5D52BA32 for ; Sat, 30 Mar 2019 08:01:39 -0400 (EDT) From: imeevma@tarantool.org Subject: [tarantool-patches] [PATCH v1 1/1] sql: parameter binding for box.execute() Date: Sat, 30 Mar 2019 15:01:37 +0300 Message-Id: <884040abe819e8ec11f7877d15e5482d6a24139b.1553947026.git.imeevma@gmail.com> 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: v.shpilevoy@tarantool.org Cc: tarantool-patches@freelists.org This patch defines parameters binding for SQL statements executed through box.execute(). Closes #3401 --- https://github.com/tarantool/tarantool/issues/3401 https://github.com/tarantool/tarantool/tree/imeevma/gh-3401-add-binding-to-box_execute src/box/execute.c | 166 +++++++++++++++++++++++++++++++ src/box/execute.h | 19 ++++ src/box/lua/init.c | 13 ++- test/sql/binding.result | 246 ++++++++++++++++++++++++++++++++++++++++++++++ test/sql/binding.test.lua | 45 +++++++++ 5 files changed, 487 insertions(+), 2 deletions(-) create mode 100644 test/sql/binding.result create mode 100644 test/sql/binding.test.lua diff --git a/src/box/execute.c b/src/box/execute.c index 6b5f9d7..f02b32d 100644 --- a/src/box/execute.c +++ b/src/box/execute.c @@ -44,6 +44,7 @@ #include "tuple.h" #include "sql/vdbe.h" #include "lua/utils.h" +#include "lua/msgpack.h" const char *sql_type_strs[] = { NULL, @@ -325,6 +326,171 @@ sql_bind_list_decode(const char *data, struct sql_bind **out_bind) return bind_count; } + +/** + * Decode a single bind column from Lua stack. + * + * @param L Lua stack. + * @param[out] bind Bind to decode to. + * @param idx Position of table with bind columns on Lua stack. + * @param i Ordinal bind number. + * + * @retval 0 Success. + * @retval -1 Memory or client error. + */ +static inline int +lua_sql_bind_decode(struct lua_State *L, struct sql_bind *bind, int idx, int i) +{ + struct luaL_field field; + struct region *region = &fiber()->gc; + char *buf; + lua_rawgeti(L, idx, i + 1); + bind->pos = i + 1; + if (lua_istable(L, -1)) { + /* + * Get key and value of the only table element to + * lua stack. + */ + lua_pushnil(L); + lua_next(L, -2); + if (! lua_isstring(L, -2)) { + diag_set(ClientError, ER_ILLEGAL_PARAMS, "name of the "\ + "parameter should be a string."); + return -1; + } + /* Check that the table is one-row sized. */ + lua_pushvalue(L, -2); + if (lua_next(L, -4) != 0) { + diag_set(ClientError, ER_ILLEGAL_PARAMS, "SQL bind "\ + "named parameter should be a table with "\ + "one key - {name = value}"); + return -1; + } + size_t name_len; + bind->name = lua_tolstring(L, -2, &name_len); + /* + * Name should be saved in allocated memory as it + * will be poped from Lua stack. + */ + buf = region_alloc(region, name_len + 1); + if (buf == NULL) { + diag_set(OutOfMemory, name_len + 1, "region_alloc", + "buf"); + return -1; + } + memcpy(buf, bind->name, name_len + 1); + bind->name = buf; + bind->name_len = name_len; + } else { + bind->name = NULL; + bind->name_len = 0; + } + if (luaL_tofield(L, luaL_msgpack_default, -1, &field) < 0) + return -1; + switch (field.type) { + case MP_UINT: + if ((uint64_t) field.ival > INT64_MAX) { + diag_set(ClientError, ER_SQL_BIND_VALUE, + sql_bind_name(bind), "INTEGER"); + return -1; + } + FALLTHROUGH; + case MP_INT: + bind->i64 = field.ival; + bind->type = SQL_INTEGER; + bind->bytes = sizeof(bind->i64); + break; + case MP_STR: + /* + * Data should be saved in allocated memory as it + * will be poped from Lua stack. + */ + buf = region_alloc(region, field.sval.len + 1); + if (buf == NULL) { + diag_set(OutOfMemory, field.sval.len + 1, + "region_alloc", "buf"); + return -1; + } + memcpy(buf, field.sval.data, field.sval.len + 1); + bind->s = buf; + bind->type = SQL_TEXT; + bind->bytes = field.sval.len; + break; + case MP_DOUBLE: + case MP_FLOAT: + bind->d = field.dval; + bind->type = SQL_FLOAT; + bind->bytes = sizeof(bind->d); + break; + case MP_NIL: + bind->type = SQL_NULL; + bind->bytes = 1; + break; + case MP_BOOL: + /* SQLite doesn't support boolean. Use int instead. */ + bind->i64 = field.bval ? 1 : 0; + bind->type = SQL_INTEGER; + bind->bytes = sizeof(bind->i64); + break; + case MP_BIN: + bind->s = mp_decode_bin(&field.sval.data, &bind->bytes); + bind->type = SQL_BLOB; + break; + case MP_EXT: + diag_set(ClientError, ER_SQL_BIND_TYPE, "USERDATA", + sql_bind_name(bind)); + return -1; + case MP_ARRAY: + diag_set(ClientError, ER_SQL_BIND_TYPE, "ARRAY", + sql_bind_name(bind)); + return -1; + case MP_MAP: + diag_set(ClientError, ER_SQL_BIND_TYPE, "MAP", + sql_bind_name(bind)); + return -1; + default: + unreachable(); + } + lua_pop(L, lua_gettop(L) - idx); + return 0; +} + +int +lua_sql_bind_list_decode(struct lua_State *L, struct sql_bind **out_bind, + int idx) +{ + assert(out_bind != NULL); + uint32_t bind_count = lua_objlen(L, idx); + if (bind_count == 0) + return 0; + if (bind_count > SQL_BIND_PARAMETER_MAX) { + diag_set(ClientError, ER_SQL_BIND_PARAMETER_MAX, + (int) bind_count); + return -1; + } + struct region *region = &fiber()->gc; + uint32_t used = region_used(region); + size_t size = sizeof(struct sql_bind) * bind_count; + /* + * Memory allocated here will be freed in + * sql_finalize() or in txn_commit()/txn_rollback() if + * there is an active transaction. + */ + struct sql_bind *bind = (struct sql_bind *) region_alloc(region, size); + if (bind == NULL) { + diag_set(OutOfMemory, size, "region_alloc", "bind"); + return -1; + } + for (uint32_t i = 0; i < bind_count; ++i) { + if (lua_sql_bind_decode(L, &bind[i], idx, i) != 0) { + region_truncate(region, used); + return -1; + } + } + *out_bind = bind; + return bind_count; +} + /** * Serialize a single column of a result set row. * @param stmt Prepared and started statement. At least one diff --git a/src/box/execute.h b/src/box/execute.h index 52563cd..bc809d8 100644 --- a/src/box/execute.h +++ b/src/box/execute.h @@ -67,6 +67,25 @@ int sql_bind_list_decode(const char *data, struct sql_bind **out_bind); /** + * Parse Lua table of SQL parameters. + * + * @param L Lua stack contains table with parameters. Each + * parameter either must have scalar type, or must be a + * single-row table with the following format: + * table[name] = value. Name - string name of the named + * parameter, value - scalar value of the parameter. + * Named and positioned parameters can be mixed. + * @param[out] out_bind Pointer to save decoded parameters. + * @param idx Position of table with parameters on Lua stack. + * + * @retval >= 0 Number of decoded parameters. + * @retval -1 Client or memory error. + */ +int +lua_sql_bind_list_decode(struct lua_State *L, struct sql_bind **out_bind, + int idx); + +/** * Prepare and execute an SQL statement. * @param sql SQL statement. * @param len Length of @a sql. diff --git a/src/box/lua/init.c b/src/box/lua/init.c index af1bcdf..cda878f 100644 --- a/src/box/lua/init.c +++ b/src/box/lua/init.c @@ -275,10 +275,19 @@ lbox_execute(struct lua_State *L) struct port port; int top = lua_gettop(L); - if (top != 1 || ! lua_isstring(L, 1)) - return luaL_error(L, "Usage: box.execute(sqlstring)"); + if (! (top == 1 || top == 2) || ! lua_isstring(L, 1)) + return luaL_error(L, "Usage: box.execute(sqlstring[, params])"); const char *sql = lua_tolstring(L, 1, &length); + + if (top == 2) { + if (! lua_istable(L, 2)) + return luaL_error(L, "Second argument must be a table"); + bind_count = lua_sql_bind_list_decode(L, &bind, 2); + if (bind_count < 0) + return luaT_error(L); + } + if (sql_prepare_and_execute(sql, length, bind, bind_count, &port, &fiber()->gc) != 0) return luaT_error(L); diff --git a/test/sql/binding.result b/test/sql/binding.result new file mode 100644 index 0000000..877d6b0 --- /dev/null +++ b/test/sql/binding.result @@ -0,0 +1,246 @@ +remote = require('net.box') +--- +... +test_run = require('test_run').new() +--- +... +engine = test_run:get_cfg('engine') +--- +... +box.execute('pragma sql_default_engine=\''..engine..'\'') +--- +- rowcount: 0 +... +box.schema.user.grant('guest','read, write, execute', 'universe') +--- +... +box.schema.user.grant('guest', 'create', 'space') +--- +... +cn = remote.connect(box.cfg.listen) +--- +... +-- gh-3401: sql.exequte arg substitution (parameter binding) +binding_values = {} +--- +... +binding_values[1] = 123 +--- +... +binding_values[2] = {} +--- +... +binding_values[2]['@value2'] = 45 +--- +... +binding_values[3] = {} +--- +... +binding_values[3][':value1'] = 67 +--- +... +box.execute('SELECT ?, ?, ?', {111, 22, 3}) +--- +- metadata: + - name: '?' + type: INTEGER + - name: '?' + type: INTEGER + - name: '?' + type: INTEGER + rows: + - [111, 22, 3] +... +box.execute('SELECT $1, $1, ?, $1, ?, $3, $2', {111, 22, 3}) +--- +- metadata: + - name: $1 + type: INTEGER + - name: $1 + type: INTEGER + - name: '?' + type: INTEGER + - name: $1 + type: INTEGER + - name: '?' + type: BOOLEAN + - name: $3 + type: BOOLEAN + - name: $2 + type: BOOLEAN + rows: + - [111, 111, 22, 111, 3, 3, 22] +... +box.execute('SELECT $3, ?', {111, 22, 3}) +--- +- metadata: + - name: $3 + type: INTEGER + - name: '?' + type: INTEGER + rows: + - [3, null] +... +box.execute('SELECT :value1, @value2', binding_values) +--- +- metadata: + - name: :value1 + type: INTEGER + - name: '@value2' + type: INTEGER + rows: + - [67, 45] +... +box.execute('SELECT ?, $1, :value1, @value2', binding_values) +--- +- metadata: + - name: '?' + type: INTEGER + - name: $1 + type: INTEGER + - name: :value1 + type: INTEGER + - name: '@value2' + type: BOOLEAN + rows: + - [123, 123, 67, 45] +... +box.execute('SELECT $1, ?, $3, :value1, @value2', binding_values) +--- +- metadata: + - name: $1 + type: INTEGER + - name: '?' + type: BOOLEAN + - name: $3 + type: BOOLEAN + - name: :value1 + type: INTEGER + - name: '@value2' + type: INTEGER + rows: + - [123, null, null, 67, 45] +... +box.execute('SELECT ?', {111, 22, 3}) +--- +- error: Bind value for parameter 2 is out of range for type INTEGER +... +box.execute('SELECT $1', {111, 22, 3}) +--- +- error: Bind value for parameter 2 is out of range for type INTEGER +... +box.execute('SELECT $1, $2, $1', {111, 22, 3}) +--- +- error: Bind value for parameter 3 is out of range for type INTEGER +... +box.execute('SELECT @value2', binding_values) +--- +- error: Parameter ':value1' was not found in the statement +... +cn:execute('SELECT ?, ?, ?', {111, 22, 3}) +--- +- metadata: + - name: '?' + type: INTEGER + - name: '?' + type: INTEGER + - name: '?' + type: INTEGER + rows: + - [111, 22, 3] +... +cn:execute('SELECT $1, $1, ?, $1, ?, $3, $2', {111, 22, 3}) +--- +- metadata: + - name: $1 + type: INTEGER + - name: $1 + type: INTEGER + - name: '?' + type: INTEGER + - name: $1 + type: INTEGER + - name: '?' + type: BOOLEAN + - name: $3 + type: BOOLEAN + - name: $2 + type: BOOLEAN + rows: + - [111, 111, 22, 111, 3, 3, 22] +... +cn:execute('SELECT $3, ?', {111, 22, 3}) +--- +- metadata: + - name: $3 + type: INTEGER + - name: '?' + type: INTEGER + rows: + - [3, null] +... +cn:execute('SELECT :value1, @value2', binding_values) +--- +- metadata: + - name: :value1 + type: INTEGER + - name: '@value2' + type: INTEGER + rows: + - [67, 45] +... +cn:execute('SELECT ?, $1, :value1, @value2', binding_values) +--- +- metadata: + - name: '?' + type: INTEGER + - name: $1 + type: INTEGER + - name: :value1 + type: INTEGER + - name: '@value2' + type: BOOLEAN + rows: + - [123, 123, 67, 45] +... +cn:execute('SELECT $1, ?, $3, :value1, @value2', binding_values) +--- +- metadata: + - name: $1 + type: INTEGER + - name: '?' + type: BOOLEAN + - name: $3 + type: BOOLEAN + - name: :value1 + type: INTEGER + - name: '@value2' + type: INTEGER + rows: + - [123, null, null, 67, 45] +... +cn:execute('SELECT ?', {111, 22, 3}) +--- +- error: Bind value for parameter 2 is out of range for type INTEGER +... +cn:execute('SELECT $1', {111, 22, 3}) +--- +- error: Bind value for parameter 2 is out of range for type INTEGER +... +cn:execute('SELECT $1, $2, $1', {111, 22, 3}) +--- +- error: Bind value for parameter 3 is out of range for type INTEGER +... +cn:execute('SELECT @value2', binding_values) +--- +- error: Parameter ':value1' was not found in the statement +... +cn:close() +--- +... +box.schema.user.revoke('guest', 'read, write, execute', 'universe') +--- +... +box.schema.user.revoke('guest', 'create', 'space') +--- +... diff --git a/test/sql/binding.test.lua b/test/sql/binding.test.lua new file mode 100644 index 0000000..577b12f --- /dev/null +++ b/test/sql/binding.test.lua @@ -0,0 +1,45 @@ +remote = require('net.box') +test_run = require('test_run').new() +engine = test_run:get_cfg('engine') +box.execute('pragma sql_default_engine=\''..engine..'\'') + +box.schema.user.grant('guest','read, write, execute', 'universe') +box.schema.user.grant('guest', 'create', 'space') +cn = remote.connect(box.cfg.listen) + +-- gh-3401: sql.exequte arg substitution (parameter binding) +binding_values = {} +binding_values[1] = 123 +binding_values[2] = {} +binding_values[2]['@value2'] = 45 +binding_values[3] = {} +binding_values[3][':value1'] = 67 + +box.execute('SELECT ?, ?, ?', {111, 22, 3}) +box.execute('SELECT $1, $1, ?, $1, ?, $3, $2', {111, 22, 3}) +box.execute('SELECT $3, ?', {111, 22, 3}) +box.execute('SELECT :value1, @value2', binding_values) +box.execute('SELECT ?, $1, :value1, @value2', binding_values) +box.execute('SELECT $1, ?, $3, :value1, @value2', binding_values) + +box.execute('SELECT ?', {111, 22, 3}) +box.execute('SELECT $1', {111, 22, 3}) +box.execute('SELECT $1, $2, $1', {111, 22, 3}) +box.execute('SELECT @value2', binding_values) + +cn:execute('SELECT ?, ?, ?', {111, 22, 3}) +cn:execute('SELECT $1, $1, ?, $1, ?, $3, $2', {111, 22, 3}) +cn:execute('SELECT $3, ?', {111, 22, 3}) +cn:execute('SELECT :value1, @value2', binding_values) +cn:execute('SELECT ?, $1, :value1, @value2', binding_values) +cn:execute('SELECT $1, ?, $3, :value1, @value2', binding_values) + +cn:execute('SELECT ?', {111, 22, 3}) +cn:execute('SELECT $1', {111, 22, 3}) +cn:execute('SELECT $1, $2, $1', {111, 22, 3}) +cn:execute('SELECT @value2', binding_values) + +cn:close() + +box.schema.user.revoke('guest', 'read, write, execute', 'universe') +box.schema.user.revoke('guest', 'create', 'space') -- 2.7.4