[tarantool-patches] [PATCH 7/8] netbox: allow passing options to :execute()

[Nikita Pettik <korablev@tarantool.org>]

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