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 E6302292BE for ; Tue, 27 Aug 2019 09:34:38 -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 UUuL-4lHlMGA for ; Tue, 27 Aug 2019 09:34:38 -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 867B929274 for ; Tue, 27 Aug 2019 09:34:38 -0400 (EDT) From: Nikita Pettik Subject: [tarantool-patches] [PATCH 7/8] netbox: allow passing options to :execute() Date: Tue, 27 Aug 2019 16:34:28 +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, kostja@tarantool.org, alexander.turenko@tarantool.org, Nikita Pettik To implement dry-run execution, let's allow 'options' be valid argument of :execute() method. SQL options are encoded with IPROTO_OPTIONS request key. Options are supposed to be a map of named parameters or a list of unnamed (considering their order). The only currently available option is 'dry_run' with boolean values. Now it doesn't affect anything, but will allow to get meta-information of query execution. Part of #3292 --- src/box/errcode.h | 1 + src/box/execute.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++ src/box/execute.h | 22 +++++++++++++++++ src/box/iproto.cc | 5 ++++ src/box/lua/net_box.lua | 3 --- src/box/xrow.c | 19 ++++++++++++--- src/box/xrow.h | 5 ++++ test/box/misc.result | 1 + test/sql/iproto.result | 18 +++++++++++++- test/sql/iproto.test.lua | 4 ++++ 10 files changed, 133 insertions(+), 7 deletions(-) diff --git a/src/box/errcode.h b/src/box/errcode.h index 46b0b365a..3aedd3995 100644 --- a/src/box/errcode.h +++ b/src/box/errcode.h @@ -253,6 +253,7 @@ struct errcode_record { /*198 */_(ER_FUNC_INDEX_FUNC, "Failed to build a key for functional index '%s' of space '%s': %s") \ /*199 */_(ER_FUNC_INDEX_FORMAT, "Key format doesn't match one defined in functional index '%s' of space '%s': %s") \ /*200 */_(ER_FUNC_INDEX_PARTS, "Wrong functional index definition: %s") \ + /*201 */_(ER_WRONG_SQL_EXECUTE_OPTIONS, "Wrong SQL options passed to execute: %.*s") \ /* * !IMPORTANT! Please follow instructions at start of the file diff --git a/src/box/execute.c b/src/box/execute.c index a8a2e516b..2e3aeef2c 100644 --- a/src/box/execute.c +++ b/src/box/execute.c @@ -51,6 +51,15 @@ const char *sql_info_key_strs[] = { "autoincrement_ids", }; +const struct sql_opts sql_opts_default = { + /* .dry_run = */ false, +}; + +const struct opt_def sql_opts_reg[] = { + OPT_DEF("dry_run", OPT_BOOL, struct sql_opts, dry_run), + OPT_END, +}; + static_assert(sizeof(struct port_sql) <= sizeof(struct port), "sizeof(struct port_sql) must be <= sizeof(struct port)"); @@ -398,6 +407,59 @@ port_sql_dump_msgpack(struct port *port, struct obuf *out) return 0; } +int +sql_opts_decode(const char *data, struct sql_opts *opts) +{ + assert(data != NULL); + assert(opts != NULL); + if (mp_typeof(*data) != MP_ARRAY) { + diag_set(ClientError, ER_INVALID_MSGPACK, + "SQL options are expected to be array"); + return -1; + } + *opts = sql_opts_default; + uint32_t opts_count = mp_decode_array(&data); + if (opts_count == 0) + return 0; + if (mp_typeof(*data) == MP_MAP) { + /* + * Options can be passed either as a map with + * named arguments: ({'dry_run' = true}); or + * as an array of unnamed values in the + * documented order. + * + * FIXME: opts_decode() as a rule is used to + * decode index or space opts. So to display + * proper diag message number of field which is + * supposed to hold options is passed as an + * argument to opts_decode(). Here we don't really + * have such field, so instead we can pass any + * meaningful and large enough constant which + * simply restricts length of error message + * (see format string of error message). + */ + if (opts_decode(opts, sql_opts_reg, &data, + ER_WRONG_SQL_EXECUTE_OPTIONS, DIAG_ERRMSG_MAX, + &fiber()->gc) != 0) + return -1; + return 0; + } + if (opts_count > 1) { + diag_set(ClientError, ER_WRONG_SQL_EXECUTE_OPTIONS, + DIAG_ERRMSG_MAX, "too many options are specified"); + return -1; + } + for (uint32_t i = 0; i < opts_count; ++i) { + if (mp_typeof(*data) != MP_BOOL) { + diag_set(ClientError, ER_WRONG_SQL_EXECUTE_OPTIONS, + DIAG_ERRMSG_MAX, "dry_run must be boolean"); + return -1; + } + opts->dry_run = mp_decode_bool(&data); + } + return 0; +} + int sql_execute(struct sql_stmt *stmt, struct port *port, struct region *region) { diff --git a/src/box/execute.h b/src/box/execute.h index 23366d65c..68aff908e 100644 --- a/src/box/execute.h +++ b/src/box/execute.h @@ -33,6 +33,7 @@ #include #include +#include "opt_def.h" #include "port.h" #if defined(__cplusplus) @@ -48,6 +49,20 @@ enum sql_info_key { extern const char *sql_info_key_strs[]; +/** Options which can be passed to :execute. */ +struct sql_opts { + /** + * In case of dry-run query is not really executed, + * but only prepared (i.e. compiled into byte-code). + * It allows to get query's metadata without execution + * or update prepared statement cache. + */ + bool dry_run; +}; + +extern const struct sql_opts sql_opts_default; +extern const struct opt_def sql_opts_reg[]; + struct region; struct sql_bind; @@ -98,6 +113,13 @@ sql_prepare(const char *sql, int length, struct sql_stmt **stmt, int sql_execute(struct sql_stmt *stmt, struct port *port, struct region *region); +/** + * Parse SQL execution options encoded in @param data and fill in + * corresponding fields in output @param opts. + */ +int +sql_opts_decode(const char *data, struct sql_opts *opts); + #if defined(__cplusplus) } /* extern "C" { */ #endif diff --git a/src/box/iproto.cc b/src/box/iproto.cc index 9b59e1af0..a92e66ace 100644 --- a/src/box/iproto.cc +++ b/src/box/iproto.cc @@ -1647,6 +1647,7 @@ tx_process_sql(struct cmsg *m) int bind_count = 0; const char *sql; uint32_t len; + struct sql_opts opts = sql_opts_default; tx_fiber_init(msg->connection->session, msg->header.sync); @@ -1659,6 +1660,10 @@ tx_process_sql(struct cmsg *m) if (bind_count < 0) goto error; } + if (msg->sql.opts != NULL) { + if (sql_opts_decode(msg->sql.opts, &opts) != 0) + goto error; + } sql = msg->sql.sql_text; sql = mp_decode_str(&sql, &len); /* Compile, bind and execute SQL statement. */ diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua index 31a8c16b7..96d68af8d 100644 --- a/src/box/lua/net_box.lua +++ b/src/box/lua/net_box.lua @@ -1185,9 +1185,6 @@ end function remote_methods:execute(query, parameters, sql_opts, netbox_opts) check_remote_arg(self, "execute") - if sql_opts ~= nil then - box.error(box.error.UNSUPPORTED, "execute", "options") - end return self:_request('execute', netbox_opts, nil, query, parameters or {}, sql_opts or {}) end diff --git a/src/box/xrow.c b/src/box/xrow.c index 0ae5271c1..5d3dc8b09 100644 --- a/src/box/xrow.c +++ b/src/box/xrow.c @@ -573,9 +573,11 @@ error: uint32_t map_size = mp_decode_map(&data); request->sql_text = NULL; request->bind = NULL; + request->opts = 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_OPTIONS) { mp_check(&data, end); /* skip the key */ mp_check(&data, end); /* skip the value */ continue; @@ -583,10 +585,21 @@ error: const char *value = ++data; /* skip the key */ if (mp_check(&data, end) != 0) /* check the value */ goto error; - if (key == IPROTO_SQL_BIND) + switch (key) { + case IPROTO_SQL_BIND: request->bind = value; - else + break; + case IPROTO_SQL_TEXT: request->sql_text = value; + break; + case IPROTO_OPTIONS: + request->opts = value; + break; + default: + xrow_on_decode_err(data, end, ER_INVALID_MSGPACK, + "unknown IPROTO request key"); + return -1; + } } if (request->sql_text == NULL) { xrow_on_decode_err(row->body[0].iov_base, end, ER_MISSING_REQUEST_FIELD, diff --git a/src/box/xrow.h b/src/box/xrow.h index 35ec06dc0..f0c1f29ed 100644 --- a/src/box/xrow.h +++ b/src/box/xrow.h @@ -527,6 +527,11 @@ struct sql_request { const char *sql_text; /** MessagePack array of parameters. */ const char *bind; + /** + * Map containing SQL execution options. + * See struct sql_options for details. + */ + const char *opts; }; /** diff --git a/test/box/misc.result b/test/box/misc.result index 287d84e5b..4d25a9fe7 100644 --- a/test/box/misc.result +++ b/test/box/misc.result @@ -530,6 +530,7 @@ t; 198: box.error.FUNC_INDEX_FUNC 199: box.error.FUNC_INDEX_FORMAT 200: box.error.FUNC_INDEX_PARTS + 201: box.error.WRONG_SQL_EXECUTE_OPTIONS ... test_run:cmd("setopt delimiter ''"); --- diff --git a/test/sql/iproto.result b/test/sql/iproto.result index 1e5c30aec..ae5349546 100644 --- a/test/sql/iproto.result +++ b/test/sql/iproto.result @@ -123,7 +123,23 @@ cn:execute(100) ... cn:execute('select 1', nil, {dry_run = true}) --- -- error: execute does not support options +- error: Tuple/Key must be MsgPack array +... +cn:execute('select 1', nil, {1}) +--- +- error: 'Wrong SQL options passed to execute: dry_run must be boolean' +... +cn:execute('select 1', nil, {true, false}) +--- +- error: 'Wrong SQL options passed to execute: too many options are specified' +... +cn:execute('select 1', nil, {{dri_run = true}}) +--- +- error: 'Wrong SQL options passed to execute: unexpected option ''dri_run''' +... +cn:execute('select 1', nil, {{dry_run = 1}}) +--- +- error: 'Wrong SQL options passed to execute: ''dry_run'' must be boolean' ... -- Empty request. cn:execute('') diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua index 5dfe95ccc..7dcea7c2e 100644 --- a/test/sql/iproto.test.lua +++ b/test/sql/iproto.test.lua @@ -39,6 +39,10 @@ cn:execute('select id as identifier from test where a = 5;') -- netbox API errors. cn:execute(100) cn:execute('select 1', nil, {dry_run = true}) +cn:execute('select 1', nil, {1}) +cn:execute('select 1', nil, {true, false}) +cn:execute('select 1', nil, {{dri_run = true}}) +cn:execute('select 1', nil, {{dry_run = 1}}) -- Empty request. cn:execute('') -- 2.15.1