Tarantool development patches archive
 help / color / mirror / Atom feed
From: imeevma@tarantool.org
To: tarantool-patches@freelists.org, v.shpilevoy@tarantool.org
Subject: [tarantool-patches] [PATCH v1 02/10] iproto: remove iproto functions from execute.c
Date: Sat, 17 Nov 2018 17:03:54 +0300	[thread overview]
Message-ID: <9c83174c116a86f4c11a1ff2ea3a16052fab3e28.1542460773.git.imeevma@gmail.com> (raw)
In-Reply-To: <cover.1542460773.git.imeevma@gmail.com>

To make functions in execute.h more universal we should reduce
their dependence on IPROTO. This patch removes IPROTO functions
from execute.c.

Needed for #3505
---
 src/box/execute.c | 116 ++++++++++++++----------------------------------------
 src/box/execute.h |  34 ++++++++--------
 src/box/iproto.cc |  71 ++++++++++++++++++++++++++++++++-
 src/box/xrow.c    |  39 ------------------
 src/box/xrow.h    |  14 -------
 5 files changed, 115 insertions(+), 159 deletions(-)

diff --git a/src/box/execute.c b/src/box/execute.c
index ce9dd83..285ae2e 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -194,23 +194,7 @@ sql_bind_decode(struct sql_bind *bind, int i, const char **packet)
 	return 0;
 }
 
-/**
- * Parse MessagePack array of SQL parameters and store a result
- * into the @request->bind, bind_count.
- * @param request Request to save decoded parameters.
- * @param data MessagePack array of parameters. Each parameter
- *        either must have scalar type, or must be a map with the
- *        following format: {name: value}. Name - string name of
- *        the named parameter, value - scalar value of the
- *        parameter. Named and positioned parameters can be mixed.
- *        For more details
- *        @sa https://www.sqlite.org/lang_expr.html#varparam.
- * @param region Allocator.
- *
- * @retval  0 Success.
- * @retval -1 Client or memory error.
- */
-static inline int
+int
 sql_bind_list_decode(struct sql_request *request, const char *data,
 		     struct region *region)
 {
@@ -246,60 +230,6 @@ sql_bind_list_decode(struct sql_request *request, const char *data,
 	return 0;
 }
 
-int
-xrow_decode_sql(const struct xrow_header *row, struct sql_request *request,
-		struct region *region)
-{
-	if (row->bodycnt == 0) {
-		diag_set(ClientError, ER_INVALID_MSGPACK, "missing request body");
-		return 1;
-	}
-	assert(row->bodycnt == 1);
-	const char *data = (const char *) row->body[0].iov_base;
-	const char *end = data + row->body[0].iov_len;
-	assert((end - data) > 0);
-
-	if (mp_typeof(*data) != MP_MAP || mp_check_map(data, end) > 0) {
-error:
-		diag_set(ClientError, ER_INVALID_MSGPACK, "packet body");
-		return -1;
-	}
-
-	uint32_t map_size = mp_decode_map(&data);
-	request->sql_text = NULL;
-	request->bind = NULL;
-	request->bind_count = 0;
-	request->sync = row->sync;
-	for (uint32_t i = 0; i < map_size; ++i) {
-		uint8_t key = *data;
-		if (key != IPROTO_SQL_BIND && key != IPROTO_SQL_TEXT) {
-			mp_check(&data, end);   /* skip the key */
-			mp_check(&data, end);   /* skip the value */
-			continue;
-		}
-		const char *value = ++data;     /* skip the key */
-		if (mp_check(&data, end) != 0)  /* check the value */
-			goto error;
-		if (key == IPROTO_SQL_BIND) {
-			if (sql_bind_list_decode(request, value, region) != 0)
-				return -1;
-		} else {
-			request->sql_text = value;
-			request->sql_text =
-				mp_decode_str(&request->sql_text,
-					      &request->sql_text_len);
-		}
-	}
-	if (request->sql_text == NULL) {
-		diag_set(ClientError, ER_MISSING_REQUEST_FIELD,
-			 iproto_key_name(IPROTO_SQL_TEXT));
-		return -1;
-	}
-	if (data != end)
-		goto error;
-	return 0;
-}
-
 /**
  * Serialize a single column of a result set row.
  * @param stmt Prepared and started statement. At least one
@@ -528,9 +458,15 @@ sql_get_description(struct sqlite3_stmt *stmt, struct obuf *out,
 		    int column_count)
 {
 	assert(column_count > 0);
-	if (iproto_reply_array_key(out, column_count, IPROTO_METADATA) != 0)
+	char *pos = (char *) obuf_alloc(out, IPROTO_KEY_HEADER_LEN);
+	if (pos == NULL) {
+		diag_set(OutOfMemory, IPROTO_KEY_HEADER_LEN, "obuf_alloc",
+			 "pos");
 		return -1;
-
+	}
+	pos = mp_store_u8(pos, IPROTO_METADATA);
+	pos = mp_store_u8(pos, 0xdd);
+	pos = mp_store_u32(pos, column_count);
 	for (int i = 0; i < column_count; ++i) {
 		size_t size = mp_sizeof_map(2) +
 			      mp_sizeof_uint(IPROTO_FIELD_NAME) +
@@ -613,27 +549,28 @@ sql_prepare_and_execute(const struct sql_request *request,
 }
 
 int
-sql_response_dump(struct sql_response *response, struct obuf *out)
+sql_response_dump(struct sql_response *response, int *keys, struct obuf *out)
 {
-	struct obuf_svp header_svp;
-	/* Prepare memory for the iproto header. */
-	if (iproto_prepare_header(out, &header_svp, IPROTO_SQL_HEADER_LEN) != 0)
-		return -1;
 	sqlite3 *db = sql_get();
 	struct sqlite3_stmt *stmt = (struct sqlite3_stmt *) response->prep_stmt;
 	struct port_tuple *port_tuple = (struct port_tuple *) &response->port;
-	int keys, rc = 0, column_count = sqlite3_column_count(stmt);
+	int rc = 0, column_count = sqlite3_column_count(stmt);
 	if (column_count > 0) {
 		if (sql_get_description(stmt, out, column_count) != 0) {
 err:
-			obuf_rollback_to_svp(out, &header_svp);
 			rc = -1;
 			goto finish;
 		}
-		keys = 2;
-		if (iproto_reply_array_key(out, port_tuple->size,
-					   IPROTO_DATA) != 0)
+		*keys = 2;
+		char *pos = (char *) obuf_alloc(out, IPROTO_KEY_HEADER_LEN);
+		if (pos == NULL) {
+			diag_set(OutOfMemory, IPROTO_KEY_HEADER_LEN,
+				 "obuf_alloc", "pos");
 			goto err;
+		}
+		pos = mp_store_u8(pos, IPROTO_DATA);
+		pos = mp_store_u8(pos, 0xdd);
+		pos = mp_store_u32(pos, port_tuple->size);
 		/*
 		 * Just like SELECT, SQL uses output format compatible
 		 * with Tarantool 1.6
@@ -643,13 +580,20 @@ err:
 			goto err;
 		}
 	} else {
-		keys = 1;
+		*keys = 1;
 		assert(port_tuple->size == 0);
 		struct stailq *autoinc_id_list =
 			vdbe_autoinc_id_list((struct Vdbe *)stmt);
 		uint32_t map_size = stailq_empty(autoinc_id_list) ? 1 : 2;
-		if (iproto_reply_map_key(out, map_size, IPROTO_SQL_INFO) != 0)
+		char *pos = (char *) obuf_alloc(out, IPROTO_KEY_HEADER_LEN);
+		if (pos == NULL) {
+			diag_set(OutOfMemory, IPROTO_KEY_HEADER_LEN,
+				 "obuf_alloc", "pos");
 			goto err;
+		}
+		pos = mp_store_u8(pos, IPROTO_SQL_INFO);
+		pos = mp_store_u8(pos, 0xdf);
+		pos = mp_store_u32(pos, map_size);
 		uint64_t id_count = 0;
 		int changes = db->nChange;
 		int size = mp_sizeof_uint(SQL_INFO_ROW_COUNT) +
@@ -683,8 +627,6 @@ err:
 			}
 		}
 	}
-	iproto_reply_sql(out, &header_svp, response->sync, schema_version,
-			 keys);
 finish:
 	port_destroy(&response->port);
 	sqlite3_finalize(stmt);
diff --git a/src/box/execute.h b/src/box/execute.h
index 79cee69..2a242e8 100644
--- a/src/box/execute.h
+++ b/src/box/execute.h
@@ -104,26 +104,34 @@ struct sql_response {
  * | }                                            |
  * +----------------------------------------------+
  * @param response EXECUTE response.
+ * @param[out] keys number of keys in dumped map.
  * @param out Output buffer.
  *
  * @retval  0 Success.
  * @retval -1 Memory error.
  */
 int
-sql_response_dump(struct sql_response *response, struct obuf *out);
+sql_response_dump(struct sql_response *response, int *keys, struct obuf *out);
 
 /**
- * Parse the EXECUTE request.
- * @param row Encoded data.
- * @param[out] request Request to decode to.
+ * Parse MessagePack array of SQL parameters and store a result
+ * into the @request->bind, bind_count.
+ * @param request Request to save decoded parameters.
+ * @param data MessagePack array of parameters. Each parameter
+ *        either must have scalar type, or must be a map with the
+ *        following format: {name: value}. Name - string name of
+ *        the named parameter, value - scalar value of the
+ *        parameter. Named and positioned parameters can be mixed.
+ *        For more details
+ *        @sa https://www.sqlite.org/lang_expr.html#varparam.
  * @param region Allocator.
  *
- * @retval  0 Sucess.
- * @retval -1 Format or memory error.
+ * @retval  0 Success.
+ * @retval -1 Client or memory error.
  */
 int
-xrow_decode_sql(const struct xrow_header *row, struct sql_request *request,
-		struct region *region);
+sql_bind_list_decode(struct sql_request *request, const char *data,
+		     struct region *region);
 
 /**
  * Prepare and execute an SQL statement.
@@ -141,16 +149,6 @@ sql_prepare_and_execute(const struct sql_request *request,
 
 #if defined(__cplusplus)
 } /* extern "C" { */
-#include "diag.h"
-
-/** @copydoc sql_request_decode. Throws on error. */
-static inline void
-xrow_decode_sql_xc(const struct xrow_header *row, struct sql_request *request,
-		   struct region *region)
-{
-	if (xrow_decode_sql(row, request, region) != 0)
-		diag_raise();
-}
 #endif
 
 #endif /* TARANTOOL_SQL_EXECUTE_H_INCLUDED */
diff --git a/src/box/iproto.cc b/src/box/iproto.cc
index cd61393..5fb2aff 100644
--- a/src/box/iproto.cc
+++ b/src/box/iproto.cc
@@ -1118,6 +1118,67 @@ static const struct cmsg_hop error_route[] = {
 	{ net_send_error, NULL },
 };
 
+/**
+ * Parse the EXECUTE request.
+ * @param row Encoded data.
+ * @param[out] request Request to decode to.
+ * @param region Allocator.
+ *
+ * @retval  0 Sucess.
+ * @retval -1 Format or memory error.
+ */
+static inline int
+xrow_decode_sql(const struct xrow_header *row, struct sql_request *request,
+		struct region *region)
+{
+	if (row->bodycnt == 0) {
+		diag_set(ClientError, ER_INVALID_MSGPACK, "missing request body");
+		return 1;
+	}
+	assert(row->bodycnt == 1);
+	const char *data = (const char *) row->body[0].iov_base;
+	const char *end = data + row->body[0].iov_len;
+	assert((end - data) > 0);
+
+	if (mp_typeof(*data) != MP_MAP || mp_check_map(data, end) > 0) {
+error:
+		diag_set(ClientError, ER_INVALID_MSGPACK, "packet body");
+		return -1;
+	}
+
+	uint32_t map_size = mp_decode_map(&data);
+	request->sql_text = NULL;
+	request->bind = NULL;
+	request->bind_count = 0;
+	request->sync = row->sync;
+	for (uint32_t i = 0; i < map_size; ++i) {
+		uint8_t key = *data;
+		if (key != IPROTO_SQL_BIND && key != IPROTO_SQL_TEXT) {
+			mp_check(&data, end);   /* skip the key */
+			mp_check(&data, end);   /* skip the value */
+			continue;
+		}
+		const char *value = ++data;     /* skip the key */
+		if (mp_check(&data, end) != 0)  /* check the value */
+			goto error;
+		if (key == IPROTO_SQL_BIND) {
+			if (sql_bind_list_decode(request, value, region) != 0)
+				return -1;
+		} else {
+			request->sql_text =
+				mp_decode_str(&value, &request->sql_text_len);
+		}
+	}
+	if (request->sql_text == NULL) {
+		diag_set(ClientError, ER_MISSING_REQUEST_FIELD,
+			 iproto_key_name(IPROTO_SQL_TEXT));
+		return -1;
+	}
+	if (data != end)
+		goto error;
+	return 0;
+}
+
 static void
 iproto_msg_decode(struct iproto_msg *msg, const char **pos, const char *reqend,
 		  bool *stop_input)
@@ -1593,8 +1654,16 @@ tx_process_sql(struct cmsg *m)
 	 * become out of date during yield.
 	 */
 	out = msg->connection->tx.p_obuf;
-	if (sql_response_dump(&response, out) != 0)
+	int keys;
+	struct obuf_svp header_svp;
+	/* Prepare memory for the iproto header. */
+	if (iproto_prepare_header(out, &header_svp, IPROTO_SQL_HEADER_LEN) != 0)
+		goto error;
+	if (sql_response_dump(&response, &keys, out) != 0) {
+		obuf_rollback_to_svp(out, &header_svp);
 		goto error;
+	}
+	iproto_reply_sql(out, &header_svp, response.sync, schema_version, keys);
 	iproto_wpos_create(&msg->wpos, out);
 	return;
 error:
diff --git a/src/box/xrow.c b/src/box/xrow.c
index 567b7ed..8a589cb 100644
--- a/src/box/xrow.c
+++ b/src/box/xrow.c
@@ -451,45 +451,6 @@ struct PACKED iproto_key_bin {
 	uint32_t mp_len;
 };
 
-/**
- * Write the key to the buffer by the specified savepoint.
- * @param buf Output buffer.
- * @param type Value type (MP_ARRAY32=0xdd, MP_MAP32=0xdf, ...).
- * @param size Value size (array or map length).
- * @param key Key value (IPROTO_DATA/DESCRIPTION/SQL_INFO).
- *
- * @retval  0 Success.
- * @retval -1 Memory error.
- */
-static inline int
-iproto_reply_key(struct obuf *buf, uint8_t type, uint32_t size, uint8_t key)
-{
-	char *pos = (char *) obuf_alloc(buf, IPROTO_KEY_HEADER_LEN);
-	if (pos == NULL) {
-		diag_set(OutOfMemory, IPROTO_KEY_HEADER_LEN, "obuf_alloc",
-			 "pos");
-		return -1;
-	}
-	struct iproto_key_bin bin;
-	bin.key = key;
-	bin.mp_type = type;
-	bin.mp_len = mp_bswap_u32(size);
-	memcpy(pos, &bin, sizeof(bin));
-	return 0;
-}
-
-int
-iproto_reply_array_key(struct obuf *buf, uint32_t size, uint8_t key)
-{
-	return iproto_reply_key(buf, 0xdd, size, key);
-}
-
-int
-iproto_reply_map_key(struct obuf *buf, uint32_t size, uint8_t key)
-{
-	return iproto_reply_key(buf, 0xdf, size, key);
-}
-
 void
 iproto_reply_select(struct obuf *buf, struct obuf_svp *svp, uint64_t sync,
 		    uint32_t schema_version, uint32_t count)
diff --git a/src/box/xrow.h b/src/box/xrow.h
index 881d860..13071ff 100644
--- a/src/box/xrow.h
+++ b/src/box/xrow.h
@@ -423,20 +423,6 @@ iproto_reply_select(struct obuf *buf, struct obuf_svp *svp, uint64_t sync,
 		    uint32_t schema_version, uint32_t count);
 
 /**
- * Write header of the key to a preallocated buffer by svp.
- * @param buf Buffer to write to.
- * @param size Size of the key (length of the array or of the
- *        string).
- * @param key Body key.
- */
-int
-iproto_reply_array_key(struct obuf *buf, uint32_t size, uint8_t key);
-
-/** @copydoc iproto_reply_array_key. */
-int
-iproto_reply_map_key(struct obuf *buf, uint32_t size, uint8_t key);
-
-/**
  * Encode iproto header with IPROTO_OK response code.
  * @param out Encode to.
  * @param sync Request sync.
-- 
2.7.4

  parent reply	other threads:[~2018-11-17 14:03 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-17 14:03 [tarantool-patches] [PATCH v1 00/10] sql: remove box.sql.execute imeevma
2018-11-17 14:03 ` [tarantool-patches] [PATCH v1 01/10] box: store sql text and length in sql_request imeevma
2018-11-17 14:03 ` imeevma [this message]
2018-11-19 17:58   ` [tarantool-patches] Re: [PATCH v1 02/10] iproto: remove iproto functions from execute.c Vladislav Shpilevoy
2018-11-17 14:03 ` [tarantool-patches] [PATCH v1 03/10] iproto: replace obuf by mpstream in execute.c imeevma
2018-11-17 14:03 ` [tarantool-patches] [PATCH v1 04/10] sql: create interface vstream imeevma
2018-11-19 17:58   ` [tarantool-patches] " Vladislav Shpilevoy
2018-11-17 14:03 ` [tarantool-patches] [PATCH v1 05/10] sql: EXPLAIN through net.box leads to SEGFAULT imeevma
2018-11-19 13:47   ` [tarantool-patches] " Vladislav Shpilevoy
2018-11-17 14:04 ` [tarantool-patches] [PATCH v1 06/10] sql: SELECT from system spaces returns unpacked msgpack imeevma
2018-11-19 13:48   ` [tarantool-patches] " Vladislav Shpilevoy
2018-11-17 14:04 ` [tarantool-patches] [PATCH v1 07/10] sql: too many autogenerated ids leads to SEGFAULT imeevma
2018-11-19 13:47   ` [tarantool-patches] " Vladislav Shpilevoy
2018-11-17 14:04 ` [tarantool-patches] [PATCH v1 08/10] box: add method dump_lua to port imeevma
2018-11-17 14:04 ` [tarantool-patches] [PATCH v1 09/10] lua: create vstream implementation for Lua imeevma
2018-11-19 17:58   ` [tarantool-patches] " Vladislav Shpilevoy
2018-11-17 14:04 ` [tarantool-patches] [PATCH v1 10/10] sql: check new box.sql.execute() imeevma
2018-11-19 12:54 ` [tarantool-patches] Re: [PATCH v1 00/10] sql: remove box.sql.execute Vladislav Shpilevoy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9c83174c116a86f4c11a1ff2ea3a16052fab3e28.1542460773.git.imeevma@gmail.com \
    --to=imeevma@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [tarantool-patches] [PATCH v1 02/10] iproto: remove iproto functions from execute.c' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox