From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 dev.tarantool.org (Postfix) with ESMTPS id C267545C31A for ; Fri, 20 Dec 2019 15:47:53 +0300 (MSK) From: Nikita Pettik Date: Fri, 20 Dec 2019 15:47:24 +0300 Message-Id: <3adc7a07e098182738681e15000a2f5ce6f700ed.1576844632.git.korablev@tarantool.org> In-Reply-To: References: In-Reply-To: References: Subject: [Tarantool-patches] [PATCH v3 19/20] netbox: introduce prepared statements List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org This patch introduces support of prepared statements in IProto protocol. To achieve this new IProto command is added - IPROTO_PREPARE (key is 0x13). It is sent with one of two mandatory keys: IPROTO_SQL_TEXT (0x40 and assumes string value) or IPROTO_STMT_ID (0x43 and assumes integer value). Depending on body it means to prepare or unprepare SQL statement: IPROTO_SQL_TEXT implies prepare request, meanwhile IPROTO_STMT_ID - unprepare. Also to reply on PREPARE request a few response keys are added: IPROTO_BIND_METADATA (0x33 and contains parameters metadata of type map) and IPROTO_BIND_COUNT (0x34 and corresponds to the count of parameters to be bound). Part of #2592 --- src/box/execute.c | 83 ++++++++++++++++++++++++++++++ src/box/iproto.cc | 68 +++++++++++++++++++++---- src/box/iproto_constants.c | 7 ++- src/box/iproto_constants.h | 5 ++ src/box/lua/net_box.c | 98 +++++++++++++++++++++++++++++++++-- src/box/lua/net_box.lua | 27 ++++++++++ src/box/xrow.c | 23 +++++++-- src/box/xrow.h | 4 +- test/box/misc.result | 1 + test/sql/engine.cfg | 1 + test/sql/iproto.result | 2 +- test/sql/prepared.result | 124 ++++++++++++++++++++++++++------------------- test/sql/prepared.test.lua | 76 +++++++++++++++++++-------- 13 files changed, 420 insertions(+), 99 deletions(-) diff --git a/src/box/execute.c b/src/box/execute.c index 09224c23a..7174d0d41 100644 --- a/src/box/execute.c +++ b/src/box/execute.c @@ -328,6 +328,68 @@ sql_get_metadata(struct sql_stmt *stmt, struct obuf *out, int column_count) return 0; } +static inline int +sql_get_params_metadata(struct sql_stmt *stmt, struct obuf *out) +{ + int bind_count = sql_bind_parameter_count(stmt); + int size = mp_sizeof_uint(IPROTO_BIND_METADATA) + + mp_sizeof_array(bind_count); + char *pos = (char *) obuf_alloc(out, size); + if (pos == NULL) { + diag_set(OutOfMemory, size, "obuf_alloc", "pos"); + return -1; + } + pos = mp_encode_uint(pos, IPROTO_BIND_METADATA); + pos = mp_encode_array(pos, bind_count); + for (int i = 0; i < bind_count; ++i) { + size_t size = mp_sizeof_map(2) + + mp_sizeof_uint(IPROTO_FIELD_NAME) + + mp_sizeof_uint(IPROTO_FIELD_TYPE); + const char *name = sql_bind_parameter_name(stmt, i); + if (name == NULL) + name = "?"; + const char *type = "ANY"; + size += mp_sizeof_str(strlen(name)); + size += mp_sizeof_str(strlen(type)); + char *pos = (char *) obuf_alloc(out, size); + if (pos == NULL) { + diag_set(OutOfMemory, size, "obuf_alloc", "pos"); + return -1; + } + pos = mp_encode_map(pos, 2); + pos = mp_encode_uint(pos, IPROTO_FIELD_NAME); + pos = mp_encode_str(pos, name, strlen(name)); + pos = mp_encode_uint(pos, IPROTO_FIELD_TYPE); + pos = mp_encode_str(pos, type, strlen(type)); + } + return 0; +} + +static int +sql_get_prepare_common_keys(struct sql_stmt *stmt, struct obuf *out, int keys) +{ + const char *sql_str = sql_stmt_query_str(stmt); + uint32_t stmt_id = sql_stmt_calculate_id(sql_str, strlen(sql_str)); + int size = mp_sizeof_map(keys) + + mp_sizeof_uint(IPROTO_STMT_ID) + + mp_sizeof_uint(stmt_id) + + mp_sizeof_uint(IPROTO_BIND_COUNT) + + mp_sizeof_uint(sql_bind_parameter_count(stmt)); + char *pos = (char *) obuf_alloc(out, size); + if (pos == NULL) { + diag_set(OutOfMemory, size, "obuf_alloc", "pos"); + return -1; + } + pos = mp_encode_map(pos, keys); + pos = mp_encode_uint(pos, IPROTO_STMT_ID); + pos = mp_encode_uint(pos, stmt_id); + pos = mp_encode_uint(pos, IPROTO_BIND_COUNT); + pos = mp_encode_uint(pos, sql_bind_parameter_count(stmt)); + if (sql_get_params_metadata(stmt, out) != 0) + return -1; + return 0; +} + static int port_sql_dump_msgpack(struct port *port, struct obuf *out) { @@ -409,6 +471,27 @@ port_sql_dump_msgpack(struct port *port, struct obuf *out) } break; } + case DQL_PREPARE: { + /* Format is following: + * query_id, + * param_count, + * params {name, type}, + * metadata {name, type} + */ + int keys = 4; + if (sql_get_prepare_common_keys(stmt, out, keys) != 0) + return -1; + return sql_get_metadata(stmt, out, sql_column_count(stmt)); + } + case DML_PREPARE: { + /* Format is following: + * query_id, + * param_count, + * params {name, type}, + */ + int keys = 3; + return sql_get_prepare_common_keys(stmt, out, keys); + } default: { unreachable(); } diff --git a/src/box/iproto.cc b/src/box/iproto.cc index c39b8e7bf..fac94658a 100644 --- a/src/box/iproto.cc +++ b/src/box/iproto.cc @@ -178,7 +178,7 @@ struct iproto_msg struct call_request call; /** Authentication request. */ struct auth_request auth; - /* SQL request, if this is the EXECUTE request. */ + /* SQL request, if this is the EXECUTE/PREPARE request. */ struct sql_request sql; /** In case of iproto parse error, saved diagnostics. */ struct diag diag; @@ -1209,6 +1209,7 @@ static const struct cmsg_hop *dml_route[IPROTO_TYPE_STAT_MAX] = { call_route, /* IPROTO_CALL */ sql_route, /* IPROTO_EXECUTE */ NULL, /* IPROTO_NOP */ + sql_route, /* IPROTO_PREPARE */ }; static const struct cmsg_hop join_route[] = { @@ -1264,6 +1265,7 @@ iproto_msg_decode(struct iproto_msg *msg, const char **pos, const char *reqend, cmsg_init(&msg->base, call_route); break; case IPROTO_EXECUTE: + case IPROTO_PREPARE: if (xrow_decode_sql(&msg->header, &msg->sql) != 0) goto error; cmsg_init(&msg->base, sql_route); @@ -1710,23 +1712,64 @@ tx_process_sql(struct cmsg *m) int bind_count = 0; const char *sql; uint32_t len; + bool is_unprepare = false; tx_fiber_init(msg->connection->session, msg->header.sync); if (tx_check_schema(msg->header.schema_version)) goto error; - assert(msg->header.type == IPROTO_EXECUTE); + assert(msg->header.type == IPROTO_EXECUTE || + msg->header.type == IPROTO_PREPARE); tx_inject_delay(); if (msg->sql.bind != NULL) { bind_count = sql_bind_list_decode(msg->sql.bind, &bind); if (bind_count < 0) goto error; } - sql = msg->sql.sql_text; - sql = mp_decode_str(&sql, &len); - if (sql_prepare_and_execute(sql, len, bind, bind_count, &port, - &fiber()->gc) != 0) - goto error; + /* + * There are four options: + * 1. Prepare SQL query (IPROTO_PREPARE + SQL string); + * 2. Unprepare SQL query (IPROTO_PREPARE + stmt id); + * 3. Execute SQL query (IPROTO_EXECUTE + SQL string); + * 4. Execute prepared query (IPROTO_EXECUTE + stmt id). + */ + if (msg->header.type == IPROTO_EXECUTE) { + if (msg->sql.sql_text != NULL) { + assert(msg->sql.stmt_id == NULL); + sql = msg->sql.sql_text; + sql = mp_decode_str(&sql, &len); + if (sql_prepare_and_execute(sql, len, bind, bind_count, + &port, &fiber()->gc) != 0) + goto error; + } else { + assert(msg->sql.sql_text == NULL); + assert(msg->sql.stmt_id != NULL); + sql = msg->sql.stmt_id; + uint32_t stmt_id = mp_decode_uint(&sql); + if (sql_execute_prepared(stmt_id, bind, bind_count, + &port, &fiber()->gc) != 0) + goto error; + } + } else { + /* IPROTO_PREPARE */ + if (msg->sql.sql_text != NULL) { + assert(msg->sql.stmt_id == NULL); + sql = msg->sql.sql_text; + sql = mp_decode_str(&sql, &len); + if (sql_prepare(sql, len, &port) != 0) + goto error; + } else { + /* UNPREPARE */ + assert(msg->sql.sql_text == NULL); + assert(msg->sql.stmt_id != NULL); + sql = msg->sql.stmt_id; + uint32_t stmt_id = mp_decode_uint(&sql); + if (sql_unprepare(stmt_id) != 0) + goto error; + is_unprepare = true; + } + } + /* * Take an obuf only after execute(). Else the buffer can * become out of date during yield. @@ -1738,12 +1781,15 @@ tx_process_sql(struct cmsg *m) port_destroy(&port); goto error; } - if (port_dump_msgpack(&port, out) != 0) { + /* Nothing to dump in case of UNPREPARE request. */ + if (! is_unprepare) { + if (port_dump_msgpack(&port, out) != 0) { + port_destroy(&port); + obuf_rollback_to_svp(out, &header_svp); + goto error; + } port_destroy(&port); - obuf_rollback_to_svp(out, &header_svp); - goto error; } - port_destroy(&port); iproto_reply_sql(out, &header_svp, msg->header.sync, schema_version); iproto_wpos_create(&msg->wpos, out); return; diff --git a/src/box/iproto_constants.c b/src/box/iproto_constants.c index 09ded1ecb..029d9888c 100644 --- a/src/box/iproto_constants.c +++ b/src/box/iproto_constants.c @@ -107,6 +107,7 @@ const char *iproto_type_strs[] = "CALL", "EXECUTE", NULL, /* NOP */ + "PREPARE", }; #define bit(c) (1ULL<sql_text = NULL; request->bind = NULL; + request->stmt_id = NULL; for (uint32_t i = 0; i < map_size; ++i) { uint8_t key = *data; - if (key != IPROTO_SQL_BIND && key != IPROTO_SQL_TEXT) { + if (key != IPROTO_SQL_BIND && key != IPROTO_SQL_TEXT && + key != IPROTO_STMT_ID) { mp_check(&data, end); /* skip the key */ mp_check(&data, end); /* skip the value */ continue; @@ -588,12 +590,23 @@ error: goto error; if (key == IPROTO_SQL_BIND) request->bind = value; - else + else if (key == IPROTO_SQL_TEXT) request->sql_text = value; + else + request->stmt_id = value; } - if (request->sql_text == NULL) { - xrow_on_decode_err(row->body[0].iov_base, end, ER_MISSING_REQUEST_FIELD, - iproto_key_name(IPROTO_SQL_TEXT)); + if (request->sql_text != NULL && request->stmt_id != NULL) { + xrow_on_decode_err(row->body[0].iov_base, end, ER_INVALID_MSGPACK, + "SQL text and statement id are incompatible "\ + "options in one request: choose one"); + return -1; + } + if (request->sql_text == NULL && request->stmt_id == NULL) { + xrow_on_decode_err(row->body[0].iov_base, end, + ER_MISSING_REQUEST_FIELD, + tt_sprintf("%s or %s", + iproto_key_name(IPROTO_SQL_TEXT), + iproto_key_name(IPROTO_STMT_ID))); return -1; } if (data != end) diff --git a/src/box/xrow.h b/src/box/xrow.h index 60def2d3c..a4d8dc015 100644 --- a/src/box/xrow.h +++ b/src/box/xrow.h @@ -526,12 +526,14 @@ int iproto_reply_error(struct obuf *out, const struct error *e, uint64_t sync, uint32_t schema_version); -/** EXECUTE request. */ +/** EXECUTE/PREPARE request. */ struct sql_request { /** SQL statement text. */ const char *sql_text; /** MessagePack array of parameters. */ const char *bind; + /** ID of prepared statement. In this case @sql_text == NULL. */ + const char *stmt_id; }; /** diff --git a/test/box/misc.result b/test/box/misc.result index 90923f28e..79fd49442 100644 --- a/test/box/misc.result +++ b/test/box/misc.result @@ -250,6 +250,7 @@ t; - EVAL - CALL - ERROR + - PREPARE - REPLACE - UPSERT - AUTH diff --git a/test/sql/engine.cfg b/test/sql/engine.cfg index a1b4b0fc5..e38bec24e 100644 --- a/test/sql/engine.cfg +++ b/test/sql/engine.cfg @@ -10,6 +10,7 @@ "local": {"remote": "false"} }, "prepared.test.lua": { + "remote": {"remote": "true"}, "local": {"remote": "false"} }, "*": { diff --git a/test/sql/iproto.result b/test/sql/iproto.result index 67acd0ac1..4dfbfce50 100644 --- a/test/sql/iproto.result +++ b/test/sql/iproto.result @@ -119,7 +119,7 @@ cn:execute('select id as identifier from test where a = 5;') -- netbox API errors. cn:execute(100) --- -- error: Syntax error near '100' +- error: Prepared statement with id 100 does not exist ... cn:execute('select 1', nil, {dry_run = true}) --- diff --git a/test/sql/prepared.result b/test/sql/prepared.result index bd37cfdd7..2f4983b00 100644 --- a/test/sql/prepared.result +++ b/test/sql/prepared.result @@ -12,34 +12,49 @@ fiber = require('fiber') -- Wrappers to make remote and local execution interface return -- same result pattern. -- -test_run:cmd("setopt delimiter ';'") +is_remote = test_run:get_cfg('remote') == 'true' | --- - | - true | ... -execute = function(...) - local res, err = box.execute(...) - if err ~= nil then - error(err) - end - return res -end; +execute = nil | --- | ... -prepare = function(...) - local res, err = box.prepare(...) - if err ~= nil then - error(err) - end - return res -end; +prepare = nil + | --- + | ... + +test_run:cmd("setopt delimiter ';'") | --- + | - true | ... -unprepare = function(...) - local res, err = box.unprepare(...) - if err ~= nil then - error(err) +if is_remote then + box.schema.user.grant('guest','read, write, execute', 'universe') + box.schema.user.grant('guest', 'create', 'space') + cn = remote.connect(box.cfg.listen) + execute = function(...) return cn:execute(...) end + prepare = function(...) return cn:prepare(...) end + unprepare = function(...) return cn:unprepare(...) end +else + execute = function(...) + local res, err = box.execute(...) + if err ~= nil then + error(err) + end + return res + end + prepare = function(...) + local res, err = box.prepare(...) + if err ~= nil then + error(err) + end + return res + end + unprepare = function(...) + local res, err = box.unprepare(...) + if err ~= nil then + error(err) + end + return res end - return res end; | --- | ... @@ -128,31 +143,26 @@ execute(s.stmt_id, {1, 3}) | type: string | rows: [] | ... -s:execute({1, 2}) + +test_run:cmd("setopt delimiter ';'") | --- - | - metadata: - | - name: ID - | type: integer - | - name: A - | type: number - | - name: B - | type: string - | rows: - | - [1, 2, '3'] + | - true | ... -s:execute({1, 3}) +if not is_remote then + res = s:execute({1, 2}) + assert(res ~= nil) + res = s:execute({1, 3}) + assert(res ~= nil) +end; | --- - | - metadata: - | - name: ID - | type: integer - | - name: A - | type: number - | - name: B - | type: string - | rows: [] | ... -s:unprepare() +test_run:cmd("setopt delimiter ''"); | --- + | - true + | ... +unprepare(s.stmt_id) + | --- + | - null | ... -- Test preparation of different types of queries. @@ -338,6 +348,7 @@ _ = prepare("SELECT a FROM test WHERE b = '3';") s = prepare("SELECT a FROM test WHERE b = '3';") | --- | ... + execute(s.stmt_id) | --- | - metadata: @@ -354,21 +365,21 @@ execute(s.stmt_id) | rows: | - [2] | ... -s:execute() +test_run:cmd("setopt delimiter ';'") | --- - | - metadata: - | - name: A - | type: number - | rows: - | - [2] + | - true | ... -s:execute() +if not is_remote then + res = s:execute() + assert(res ~= nil) + res = s:execute() + assert(res ~= nil) +end; | --- - | - metadata: - | - name: A - | type: number - | rows: - | - [2] + | ... +test_run:cmd("setopt delimiter ''"); + | --- + | - true | ... unprepare(s.stmt_id) | --- @@ -671,6 +682,13 @@ unprepare(s.stmt_id); | - null | ... +if is_remote then + cn:close() + box.schema.user.revoke('guest', 'read, write, execute', 'universe') + box.schema.user.revoke('guest', 'create', 'space') +end; + | --- + | ... test_run:cmd("setopt delimiter ''"); | --- | - true diff --git a/test/sql/prepared.test.lua b/test/sql/prepared.test.lua index 49d2fb3ae..c464cc21a 100644 --- a/test/sql/prepared.test.lua +++ b/test/sql/prepared.test.lua @@ -5,27 +5,40 @@ fiber = require('fiber') -- Wrappers to make remote and local execution interface return -- same result pattern. -- +is_remote = test_run:get_cfg('remote') == 'true' +execute = nil +prepare = nil + test_run:cmd("setopt delimiter ';'") -execute = function(...) - local res, err = box.execute(...) - if err ~= nil then - error(err) +if is_remote then + box.schema.user.grant('guest','read, write, execute', 'universe') + box.schema.user.grant('guest', 'create', 'space') + cn = remote.connect(box.cfg.listen) + execute = function(...) return cn:execute(...) end + prepare = function(...) return cn:prepare(...) end + unprepare = function(...) return cn:unprepare(...) end +else + execute = function(...) + local res, err = box.execute(...) + if err ~= nil then + error(err) + end + return res end - return res -end; -prepare = function(...) - local res, err = box.prepare(...) - if err ~= nil then - error(err) + prepare = function(...) + local res, err = box.prepare(...) + if err ~= nil then + error(err) + end + return res end - return res -end; -unprepare = function(...) - local res, err = box.unprepare(...) - if err ~= nil then - error(err) + unprepare = function(...) + local res, err = box.unprepare(...) + if err ~= nil then + error(err) + end + return res end - return res end; test_run:cmd("setopt delimiter ''"); @@ -46,9 +59,16 @@ s.params s.param_count execute(s.stmt_id, {1, 2}) execute(s.stmt_id, {1, 3}) -s:execute({1, 2}) -s:execute({1, 3}) -s:unprepare() + +test_run:cmd("setopt delimiter ';'") +if not is_remote then + res = s:execute({1, 2}) + assert(res ~= nil) + res = s:execute({1, 3}) + assert(res ~= nil) +end; +test_run:cmd("setopt delimiter ''"); +unprepare(s.stmt_id) -- Test preparation of different types of queries. -- Let's start from DDL. It doesn't make much sense since @@ -111,10 +131,17 @@ space:replace{4, 5, '6'} space:replace{7, 8.5, '9'} _ = prepare("SELECT a FROM test WHERE b = '3';") s = prepare("SELECT a FROM test WHERE b = '3';") + execute(s.stmt_id) execute(s.stmt_id) -s:execute() -s:execute() +test_run:cmd("setopt delimiter ';'") +if not is_remote then + res = s:execute() + assert(res ~= nil) + res = s:execute() + assert(res ~= nil) +end; +test_run:cmd("setopt delimiter ''"); unprepare(s.stmt_id) s = prepare("SELECT count(*), count(a - 3), max(b), abs(id) FROM test WHERE b = '3';") @@ -233,6 +260,11 @@ f2:join(); unprepare(s.stmt_id); +if is_remote then + cn:close() + box.schema.user.revoke('guest', 'read, write, execute', 'universe') + box.schema.user.revoke('guest', 'create', 'space') +end; test_run:cmd("setopt delimiter ''"); box.cfg{sql_cache_size = 5 * 1024 * 1024} -- 2.15.1