[tarantool-patches] [PATCH 7/8] netbox: allow passing options to :execute()
Nikita Pettik
korablev at tarantool.org
Tue Aug 27 16:34:28 MSK 2019
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 <stdint.h>
#include <stdbool.h>
+#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
More information about the Tarantool-patches
mailing list