[tarantool-patches] [PATCH v7 3/6] iproto: create port_sql

[imeevma@tarantool.org]

Hi! Thank you for review! My answers, diff between versions and
new patch below.

On 12/29/18 4:19 PM, Vladislav Shpilevoy wrote:
> Thanks for the patch! See 11 comments below.
>
> On 28/12/2018 21:11, imeevma@tarantool.org wrote:
>> Hi! Thank you for review! My answers, diff between versions and
>> new version below.
>>
>> On 12/25/18 5:57 PM, Vladislav Shpilevoy wrote:
>>> Hi! Thanks for the patch! See 4 comments below. 
>>
>>> 1. Please, write a comment about how could you use port_tuple_vtab on
>>> port_sql. Here I expected to see words about calling methods of a base
>>> structure, like BaseClass::method in C++. 
>> Added. Not sure that it is enough. 
>
> Than add more explanations? Hardly it is possible to 'overexplain'
> something, but it is as easy as nothing to 'underexplain'.
>
Changed comment. I think that should be enough.

>>
>>> 2. Please, rename this method to port_sql_create and call here port_tuple_create()
>>> explicitly. It is just a constructor. port_tuple_add, used in sql_execute(), will
>>> work anyway. Since port_sql is derived from port_tuple, it can be used wherever the
>>> base. 
>> Done.
>>
>>> 3. I think, we should not expose port_sql to the header. It is used in execute.c
>>> only. Same about port_tuple_to_port_sql. 
>> Done.
>>
>>> 4. Do not forget about comments. It still describes 'response' parameter. 
>> Fixed.
>>
>>
>> Diff between versions:
>>
>> commit 414bf16c094fbb7b9e0ecb6b4b56f069f109ef86
>> Author: Mergen Imeev <imeevma@gmail.com>
>> Date:   Wed Dec 26 22:00:35 2018 +0300
>>
>>      Temporary: review fixes
>>
>> diff --git a/src/box/execute.c b/src/box/execute.c
>> index b07de28..c6fcb50 100644
>> --- a/src/box/execute.c
>> +++ b/src/box/execute.c
>> @@ -83,6 +83,36 @@ struct sql_bind {
>>   };
>>     /**
>> + * Port implementation used for dump tuples, stored in port_tuple,
>> + * to obuf or Lua. 
>
> 1. Not only tuples, but the whole response, including headers,
> meta, info.
>
Fixed.

>> + *
>> + * All port_tuple methods can be used with port_sql, since
>> + * port_sql is a structure inherited from port_tuple. Calling
>> + * port_tuple methods we are going via port_tuple_vtab. 
>
> 2. Not sure if the sentence is correct. 'Calling we are going'
> looks flawed. Maybe better 'Port_tuple methods are called via
> explicit access to port_tuple_vtab just like C++ does with
> BaseClass::method, when it is called in a child method'. ?
> Or something like this.
>
Fixed.

>> + */
>> +struct port_sql {
>> +    /* port_tuple to inherit from. */
>> +    struct port_tuple port_tuple;
>> +    /* Prepared SQL statement. */
>> +    struct sqlite3_stmt *stmt;
>> +};
>> +static_assert(sizeof(struct port_sql) <= sizeof(struct port),
>> +          "sizeof(struct port_sql) must be <= sizeof(struct port)");
>> +
>> +static int
>> +port_sql_dump_msgpack(struct port *port, struct obuf *out); 
>
> 3. Please, add empty line between function declarations. Also, we
> usually write a comment above a first function declaration appearance,
> so the whole comment about port_sql_dump_msgpack() should be put here.
>
Fixed.

>> +static void
>> +port_sql_destroy(struct port *base); 
>
> 4. Correct me if I am wrong, but as I see, port_sql_destroy can be
> implemented right here, without pre-announcement.
You are right, fixed.

>
> Also, please, move port_sql_create here to gather all methods
> in one place.
>
Fixed.

>> +
>> +const struct port_vtab port_sql_vtab = { 
>
> 5. 'static'. It is not used anywhere outside execute.c now.
>
Fixed.

>> +    .dump_msgpack = port_sql_dump_msgpack,
>> +    .dump_msgpack_16 = NULL,
>> +    .dump_lua = NULL,
>> +    .dump_plain = NULL,
>> +    .destroy = port_sql_destroy, 
>
> 6. Not sure why, but in new code we do not explicitly
> write attribute names. Instead, we use
>
>     /* .method_name = */ func,
>
> not
>
>     .method_name = func,
>
> To be honest I like your way and how it was done in old
> code, but Vladimir once forced me to use /* ... = */, so
> lets follow.
>
Fixed.

>> +};
>> +
>> +/**
>>    * Return a string name of a parameter marker.
>>    * @param Bind to get name.
>>    * @retval Zero terminated name.
>> @@ -356,6 +386,11 @@ sql_row_to_port(struct sqlite3_stmt *stmt, int column_count,
>>       if (tuple == NULL)
>>           goto error;
>>       region_truncate(region, svp);
>> +    /*
>> +     * The port_tuple_add function can be used with port_sql,
>> +     * since it does not call any port_tuple methods and works
>> +     * only with fields. 
>
> 7. It does not matter even if it would call port_tuple_methods.
> As we stated in port_sql comment, we derived from port_tuple and
> can use it in-place of base. I do not think this particular place
> should be commented at all.
>
Removed all such comments with exception of one before definition
of struct port_sql.

>> +     */
>>       return port_tuple_add(port, tuple);
>>     error:
>> commit 8390b09d95aa5bcaa6ef89924d9a334dff746f19
>> Author: Mergen Imeev <imeevma@gmail.com>
>> Date:   Fri Dec 21 14:52:03 2018 +0300
>>
>>      iproto: create port_sql
>>           This patch creates port_sql implementation for the port. This will
>>      allow us to dump sql responses to obuf or to Lua. Also this patch
>>      defines methods dump_msgpack() and destroy() of port_sql.
>>           Part of #3505
>>
>> diff --git a/src/box/execute.c b/src/box/execute.c
>> index 38b6cbc..c6fcb50 100644
>> --- a/src/box/execute.c
>> +++ b/src/box/execute.c
>> @@ -575,26 +606,19 @@ err:
>>               rc = -1;
>>               goto finish;
>>           }
>> -        size = mp_sizeof_uint(IPROTO_DATA) +
>> -               mp_sizeof_array(port_tuple->size);
>> +        size = mp_sizeof_uint(IPROTO_DATA);
>>           pos = (char *) obuf_alloc(out, size);
>>           if (pos == NULL) {
>>               diag_set(OutOfMemory, size, "obuf_alloc", "pos");
>>               goto err;
>>           }
>>           pos = mp_encode_uint(pos, IPROTO_DATA);
>> -        pos = mp_encode_array(pos, port_tuple->size);
>> -        /*
>> -         * Just like SELECT, SQL uses output format compatible
>> -         * with Tarantool 1.6
>> -         */
>> -        if (port_dump_msgpack_16(&response->port, out) < 0) {
>> -            /* Failed port dump destroyes the port. */
>> +        /* Calling BaseStruct::methods via its vtab. */ 
>
> 8. Lets do not repeat this comment on each port_tuple_vtab usage. It
> it direct repetition of what is said in port_sql comment.
>
Fixed.

>> +        if (port_tuple_vtab.dump_msgpack(port, out) < 0)
>>               goto err;
>> -        }
>>       } else {
>>           int keys = 1;
>> -        assert(port_tuple->size == 0);
>> +        assert(((struct port_tuple *)port)->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;
>> @@ -643,7 +667,63 @@ err:
>> +
>> +static inline int
>> +sql_execute(sqlite3 *db, struct sqlite3_stmt *stmt, struct port *port,
>> +        struct region *region) 
>
> 9. The function is not so trivial to omit a comment. Please, write it.
> In particular what a one should do with region after sql_execute() is
> finished. Should it be cleared?
>
Added description.

>> +{
>> +    int rc, column_count = sqlite3_column_count(stmt);
>> +    if (column_count > 0) {
>> +        /* Either ROW or DONE or ERROR. */
>> +        while ((rc = sqlite3_step(stmt)) == SQLITE_ROW) {
>> +            if (sql_row_to_port(stmt, column_count, region,
>> +                        port) != 0)
>> +                return -1;
>> +        }
>> +        assert(rc == SQLITE_DONE || rc != SQLITE_OK);
>> +    } else {
>> +        /* No rows. Either DONE or ERROR. */
>> +        rc = sqlite3_step(stmt);
>> +        assert(rc != SQLITE_ROW && rc != SQLITE_OK);
>> +    }
>> +    if (rc != SQLITE_DONE) {
>> +        diag_set(ClientError, ER_SQL_EXECUTE, sqlite3_errmsg(db));
>> +        return -1;
>> +    }
>> +    return 0;
>> +}
>> +
>> +int
>> +sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind,
>> +            uint32_t bind_count, struct port *port,
>> +            struct region *region)
>> +{
>> +    struct sqlite3_stmt *stmt;
>> +    sqlite3 *db = sql_get();
>> +    if (db == NULL) {
>> +        diag_set(ClientError, ER_LOADING);
>> +        return -1; 
>
> 10. Not sure if it is possible. Is it?
>
Removed.

>> +    }
>> +    if (sqlite3_prepare_v2(db, sql, len, &stmt, NULL) != SQLITE_OK) {
>> +        diag_set(ClientError, ER_SQL_EXECUTE, sqlite3_errmsg(db));
>> +        return -1;
>> +    }
>> +    assert(stmt != NULL);
>> +    port_sql_create(port, stmt);
>> +    if (sql_bind(stmt, bind, bind_count) == 0 &&
>> +        sql_execute(db, stmt, port, region) == 0)
>> +        return 0;
>> +    port_destroy(port);
>> +    return -1;
>> +}
>> diff --git a/src/box/iproto.cc b/src/box/iproto.cc
>> index a08c8c5..9dc0462 100644
>> --- a/src/box/iproto.cc
>> +++ b/src/box/iproto.cc
>> @@ -1645,7 +1645,7 @@ tx_process_sql(struct cmsg *m)
>>       /* Prepare memory for the iproto header. */
>>       if (iproto_prepare_header(out, &header_svp, IPROTO_HEADER_LEN) != 0)
>>           goto error;
>> -    if (sql_response_dump(&response, out) != 0) {
>> +    if (port_dump_msgpack(&port, out) != 0) {
>>           obuf_rollback_to_svp(out, &header_svp);
>>           goto error; 
>
> 11. Port leaks if an error occurred in iproto_prepare_header.
>
Fixed.

>>       } 
>
> Sorry, most of comments are minor and I could fix them
> myself, but I was asked to do not rewrite patches if they
> are not urgent. 


Diff between versions:

commit 20864423db67a3322e203cb69d910d3979d63c2b
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Wed Jan 9 17:36:15 2019 +0300

    Temporary: review fix

diff --git a/src/box/execute.c b/src/box/execute.c
index c6fcb50..a81f482 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -83,12 +83,15 @@ struct sql_bind {
 };
 
 /**
- * Port implementation used for dump tuples, stored in port_tuple,
- * to obuf or Lua.
+ * Port implementation that is used to store SQL responses and
+ * output them to obuf or Lua. This port implementation is
+ * inherited from the port_tuple structure. This allows us to use
+ * this structure in the port_tuple methods instead of port_tuple
+ * itself.
  *
- * All port_tuple methods can be used with port_sql, since
- * port_sql is a structure inherited from port_tuple. Calling
- * port_tuple methods we are going via port_tuple_vtab.
+ * The methods of port_tuple are called via explicit access to
+ * port_tuple_vtab just like C++ does with BaseClass::method, when
+ * it is called in a child method.
  */
 struct port_sql {
  /* port_tuple to inherit from. */
@@ -96,22 +99,76 @@ struct port_sql {
  /* Prepared SQL statement. */
  struct sqlite3_stmt *stmt;
 };
+
 static_assert(sizeof(struct port_sql) <= sizeof(struct port),
        "sizeof(struct port_sql) must be <= sizeof(struct port)");
 
+/**
+ * Dump data from port to buffer. Data in port contains tuples,
+ * metadata, or information obtained from an executed SQL query.
+ * The port is destroyed.
+ *
+ * Dumped msgpack structure:
+ * +----------------------------------------------+
+ * | IPROTO_BODY: {                               |
+ * |     IPROTO_METADATA: [                       |
+ * |         {IPROTO_FIELD_NAME: column name1},   |
+ * |         {IPROTO_FIELD_NAME: column name2},   |
+ * |         ...                                  |
+ * |     ],                                       |
+ * |                                              |
+ * |     IPROTO_DATA: [                           |
+ * |         tuple, tuple, tuple, ...             |
+ * |     ]                                        |
+ * | }                                            |
+ * +-------------------- OR ----------------------+
+ * | IPROTO_BODY: {                               |
+ * |     IPROTO_SQL_INFO: {                       |
+ * |         SQL_INFO_ROW_COUNT: number           |
+ * |         SQL_INFO_AUTOINCREMENT_IDS: [        |
+ * |             id, id, id, ...                  |
+ * |         ]                                    |
+ * |     }                                        |
+ * | }                                            |
+ * +-------------------- OR ----------------------+
+ * | IPROTO_BODY: {                               |
+ * |     IPROTO_SQL_INFO: {                       |
+ * |         SQL_INFO_ROW_COUNT: number           |
+ * |     }                                        |
+ * | }                                            |
+ * +----------------------------------------------+
+ * @param port Port that contains SQL response.
+ * @param[out] out Output buffer.
+ *
+ * @retval  0 Success.
+ * @retval -1 Memory error.
+ */
 static int
 port_sql_dump_msgpack(struct port *port, struct obuf *out);
+
 static void
-port_sql_destroy(struct port *base);
+port_sql_destroy(struct port *base)
+{
+ port_tuple_vtab.destroy(base);
+ sqlite3_finalize(((struct port_sql *)base)->stmt);
+}
 
-const struct port_vtab port_sql_vtab = {
- .dump_msgpack = port_sql_dump_msgpack,
- .dump_msgpack_16 = NULL,
- .dump_lua = NULL,
- .dump_plain = NULL,
- .destroy = port_sql_destroy,
+static const struct port_vtab port_sql_vtab = {
+ /* .dump_msgpack = */ port_sql_dump_msgpack,
+ /* .dump_msgpack_16 = */ NULL,
+ /* .dump_lua = */ NULL,
+ /* .dump_plain = */ NULL,
+ /* .destroy = */ port_sql_destroy,
 };
 
+static void
+port_sql_create(struct port *port, struct sqlite3_stmt *stmt)
+{
+ port_tuple_create(port);
+ ((struct port_sql *)port)->stmt = stmt;
+ port->vtab = &port_sql_vtab;
+}
+
 /**
  * Return a string name of a parameter marker.
  * @param Bind to get name.
@@ -386,11 +443,6 @@ sql_row_to_port(struct sqlite3_stmt *stmt, int column_count,
  if (tuple == NULL)
    goto error;
  region_truncate(region, svp);
- /*
-  * The port_tuple_add function can be used with port_sql,
-  * since it does not call any port_tuple methods and works
-  * only with fields.
-  */
  return port_tuple_add(port, tuple);
 
 error:
@@ -522,6 +574,7 @@ sql_get_description(struct sqlite3_stmt *stmt, struct obuf *out,
     * column_name simply returns them.
     */
    assert(name != NULL);
+   assert(type != NULL);
    size += mp_sizeof_str(strlen(name));
    size += mp_sizeof_str(strlen(type));
    char *pos = (char *) obuf_alloc(out, size);
@@ -538,53 +591,6 @@ sql_get_description(struct sqlite3_stmt *stmt, struct obuf *out,
  return 0;
 }
 
-static void
-port_sql_create(struct port *port, struct sqlite3_stmt *stmt)
-{
- port_tuple_create(port);
- ((struct port_sql *)port)->stmt = stmt;
- port->vtab = &port_sql_vtab;
-}
-
-/**
- * Dump tuples, metadata, or information obtained from an excuted
- * SQL query and saved to the port in obuf. The port is destroyed.
- *
- * Dumped msgpack structure:
- * +----------------------------------------------+
- * | IPROTO_BODY: {                               |
- * |     IPROTO_METADATA: [                       |
- * |         {IPROTO_FIELD_NAME: column name1},   |
- * |         {IPROTO_FIELD_NAME: column name2},   |
- * |         ...                                  |
- * |     ],                                       |
- * |                                              |
- * |     IPROTO_DATA: [                           |
- * |         tuple, tuple, tuple, ...             |
- * |     ]                                        |
- * | }                                            |
- * +-------------------- OR ----------------------+
- * | IPROTO_BODY: {                               |
- * |     IPROTO_SQL_INFO: {                       |
- * |         SQL_INFO_ROW_COUNT: number           |
- * |         SQL_INFO_AUTOINCREMENT_IDS: [        |
- * |             id, id, id, ...                  |
- * |         ]                                    |
- * |     }                                        |
- * | }                                            |
- * +-------------------- OR ----------------------+
- * | IPROTO_BODY: {                               |
- * |     IPROTO_SQL_INFO: {                       |
- * |         SQL_INFO_ROW_COUNT: number           |
- * |     }                                        |
- * | }                                            |
- * +----------------------------------------------+
- * @param port port with tuples, metadata or info.
- * @param out Output buffer.
- *
- * @retval  0 Success.
- * @retval -1 Memory error.
- */
 static int
 port_sql_dump_msgpack(struct port *port, struct obuf *out)
 {
@@ -613,7 +619,6 @@ err:
      goto err;
    }
    pos = mp_encode_uint(pos, IPROTO_DATA);
-   /* Calling BaseStruct::methods via its vtab. */
    if (port_tuple_vtab.dump_msgpack(port, out) < 0)
      goto err;
  } else {
@@ -671,14 +676,21 @@ finish:
  return rc;
 }
 
-static void
-port_sql_destroy(struct port *base)
-{
- /* Calling BaseStruct::methods via its vtab. */
- port_tuple_vtab.destroy(base);
- sqlite3_finalize(((struct port_sql *)base)->stmt);
-}
-
+/**
+ * Execute prepared SQL statement.
+ *
+ * This function uses region to allocate memory for temporary
+ * objects. After this function, region will be in the same state
+ * in which it was before this function.
+ *
+ * @param db SQL handle.
+ * @param stmt Prepared statement.
+ * @param[out] port Port to store SQL response.
+ * @param region Region to allocate temporary objects.
+ *
+ * @retval  0 Success.
+ * @retval -1 Error.
+ */
 static inline int
 sql_execute(sqlite3 *db, struct sqlite3_stmt *stmt, struct port *port,
      struct region *region)
@@ -711,10 +723,6 @@ sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind,
 {
  struct sqlite3_stmt *stmt;
  sqlite3 *db = sql_get();
- if (db == NULL) {
-   diag_set(ClientError, ER_LOADING);
-   return -1;
- }
  if (sqlite3_prepare_v2(db, sql, len, &stmt, NULL) != SQLITE_OK) {
    diag_set(ClientError, ER_SQL_EXECUTE, sqlite3_errmsg(db));
    return -1;
diff --git a/src/box/execute.h b/src/box/execute.h
index c90789f..1ed9ab5 100644
--- a/src/box/execute.h
+++ b/src/box/execute.h
@@ -74,7 +74,7 @@ sql_bind_list_decode(const char *data, struct sql_bind **out_bind);
  * @param len Length of @a sql.
  * @param bind Array of parameters.
  * @param bind_count Length of @a bind.
- * @param[out] response Response to store result.
+ * @param[out] port Port to store SQL response.
  * @param region Runtime allocator for temporary objects
  *        (columns, tuples ...).
  *
diff --git a/src/box/iproto.cc b/src/box/iproto.cc
index 9dc0462..09e8914 100644
--- a/src/box/iproto.cc
+++ b/src/box/iproto.cc
@@ -1643,8 +1643,10 @@ tx_process_sql(struct cmsg *m)
  out = msg->connection->tx.p_obuf;
  struct obuf_svp header_svp;
  /* Prepare memory for the iproto header. */
- if (iproto_prepare_header(out, &header_svp, IPROTO_HEADER_LEN) != 0)
+ if (iproto_prepare_header(out, &header_svp, IPROTO_HEADER_LEN) != 0) {
+   port_destroy(&port);
    goto error;
+ }
  if (port_dump_msgpack(&port, out) != 0) {
    obuf_rollback_to_svp(out, &header_svp);
    goto error;


New version:

commit 1a3007030ea54fa81533acbf7421f27a2399c941
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Fri Dec 21 14:52:03 2018 +0300

    iproto: create port_sql
    
    This patch creates port_sql implementation for the port. This will
    allow us to dump sql responses to obuf or to Lua. Also this patch
    defines methods dump_msgpack() and destroy() of port_sql.
    
    Part of #3505

diff --git a/src/box/execute.c b/src/box/execute.c
index 38b6cbc..a81f482 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -83,6 +83,93 @@ struct sql_bind {
 };
 
 /**
+ * Port implementation that is used to store SQL responses and
+ * output them to obuf or Lua. This port implementation is
+ * inherited from the port_tuple structure. This allows us to use
+ * this structure in the port_tuple methods instead of port_tuple
+ * itself.
+ *
+ * The methods of port_tuple are called via explicit access to
+ * port_tuple_vtab just like C++ does with BaseClass::method, when
+ * it is called in a child method.
+ */
+struct port_sql {
+ /* port_tuple to inherit from. */
+ struct port_tuple port_tuple;
+ /* Prepared SQL statement. */
+ struct sqlite3_stmt *stmt;
+};
+
+static_assert(sizeof(struct port_sql) <= sizeof(struct port),
+       "sizeof(struct port_sql) must be <= sizeof(struct port)");
+
+/**
+ * Dump data from port to buffer. Data in port contains tuples,
+ * metadata, or information obtained from an executed SQL query.
+ * The port is destroyed.
+ *
+ * Dumped msgpack structure:
+ * +----------------------------------------------+
+ * | IPROTO_BODY: {                               |
+ * |     IPROTO_METADATA: [                       |
+ * |         {IPROTO_FIELD_NAME: column name1},   |
+ * |         {IPROTO_FIELD_NAME: column name2},   |
+ * |         ...                                  |
+ * |     ],                                       |
+ * |                                              |
+ * |     IPROTO_DATA: [                           |
+ * |         tuple, tuple, tuple, ...             |
+ * |     ]                                        |
+ * | }                                            |
+ * +-------------------- OR ----------------------+
+ * | IPROTO_BODY: {                               |
+ * |     IPROTO_SQL_INFO: {                       |
+ * |         SQL_INFO_ROW_COUNT: number           |
+ * |         SQL_INFO_AUTOINCREMENT_IDS: [        |
+ * |             id, id, id, ...                  |
+ * |         ]                                    |
+ * |     }                                        |
+ * | }                                            |
+ * +-------------------- OR ----------------------+
+ * | IPROTO_BODY: {                               |
+ * |     IPROTO_SQL_INFO: {                       |
+ * |         SQL_INFO_ROW_COUNT: number           |
+ * |     }                                        |
+ * | }                                            |
+ * +----------------------------------------------+
+ * @param port Port that contains SQL response.
+ * @param[out] out Output buffer.
+ *
+ * @retval  0 Success.
+ * @retval -1 Memory error.
+ */
+static int
+port_sql_dump_msgpack(struct port *port, struct obuf *out);
+
+static void
+port_sql_destroy(struct port *base)
+{
+ port_tuple_vtab.destroy(base);
+ sqlite3_finalize(((struct port_sql *)base)->stmt);
+}
+
+static const struct port_vtab port_sql_vtab = {
+ /* .dump_msgpack = */ port_sql_dump_msgpack,
+ /* .dump_msgpack_16 = */ NULL,
+ /* .dump_lua = */ NULL,
+ /* .dump_plain = */ NULL,
+ /* .destroy = */ port_sql_destroy,
+};
+
+static void
+port_sql_create(struct port *port, struct sqlite3_stmt *stmt)
+{
+ port_tuple_create(port);
+ ((struct port_sql *)port)->stmt = stmt;
+ port->vtab = &port_sql_vtab;
+}
+
+/**
  * Return a string name of a parameter marker.
  * @param Bind to get name.
  * @retval Zero terminated name.
@@ -487,6 +574,7 @@ sql_get_description(struct sqlite3_stmt *stmt, struct obuf *out,
     * column_name simply returns them.
     */
    assert(name != NULL);
+   assert(type != NULL);
    size += mp_sizeof_str(strlen(name));
    size += mp_sizeof_str(strlen(type));
    char *pos = (char *) obuf_alloc(out, size);
@@ -503,63 +591,12 @@ sql_get_description(struct sqlite3_stmt *stmt, struct obuf *out,
  return 0;
 }
 
-static inline int
-sql_execute(sqlite3 *db, struct sqlite3_stmt *stmt, struct port *port,
-     struct region *region)
+static int
+port_sql_dump_msgpack(struct port *port, struct obuf *out)
 {
- int rc, column_count = sqlite3_column_count(stmt);
- if (column_count > 0) {
-   /* Either ROW or DONE or ERROR. */
-   while ((rc = sqlite3_step(stmt)) == SQLITE_ROW) {
-     if (sql_row_to_port(stmt, column_count, region,
-             port) != 0)
-       return -1;
-   }
-   assert(rc == SQLITE_DONE || rc != SQLITE_OK);
- } else {
-   /* No rows. Either DONE or ERROR. */
-   rc = sqlite3_step(stmt);
-   assert(rc != SQLITE_ROW && rc != SQLITE_OK);
- }
- if (rc != SQLITE_DONE) {
-   diag_set(ClientError, ER_SQL_EXECUTE, sqlite3_errmsg(db));
-   return -1;
- }
- return 0;
-}
-
-int
-sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind,
-     uint32_t bind_count, struct sql_response *response,
-     struct region *region)
-{
- struct sqlite3_stmt *stmt;
+ assert(port->vtab == &port_sql_vtab);
  sqlite3 *db = sql_get();
- if (db == NULL) {
-   diag_set(ClientError, ER_LOADING);
-   return -1;
- }
- if (sqlite3_prepare_v2(db, sql, len, &stmt, NULL) != SQLITE_OK) {
-   diag_set(ClientError, ER_SQL_EXECUTE, sqlite3_errmsg(db));
-   return -1;
- }
- assert(stmt != NULL);
- port_tuple_create(&response->port);
- response->prep_stmt = stmt;
- if (sql_bind(stmt, bind, bind_count) == 0 &&
-     sql_execute(db, stmt, &response->port, region) == 0)
-   return 0;
- port_destroy(&response->port);
- sqlite3_finalize(stmt);
- return -1;
-}
-
-int
-sql_response_dump(struct sql_response *response, struct obuf *out)
-{
- sqlite3 *db = sql_get();
- struct sqlite3_stmt *stmt = (struct sqlite3_stmt *) response->prep_stmt;
- struct port_tuple *port_tuple = (struct port_tuple *) &response->port;
+ struct sqlite3_stmt *stmt = ((struct port_sql *)port)->stmt;
  int rc = 0, column_count = sqlite3_column_count(stmt);
  if (column_count > 0) {
    int keys = 2;
@@ -575,26 +612,18 @@ err:
      rc = -1;
      goto finish;
    }
-   size = mp_sizeof_uint(IPROTO_DATA) +
-          mp_sizeof_array(port_tuple->size);
+   size = mp_sizeof_uint(IPROTO_DATA);
    pos = (char *) obuf_alloc(out, size);
    if (pos == NULL) {
      diag_set(OutOfMemory, size, "obuf_alloc", "pos");
      goto err;
    }
    pos = mp_encode_uint(pos, IPROTO_DATA);
-   pos = mp_encode_array(pos, port_tuple->size);
-   /*
-    * Just like SELECT, SQL uses output format compatible
-    * with Tarantool 1.6
-    */
-   if (port_dump_msgpack_16(&response->port, out) < 0) {
-     /* Failed port dump destroyes the port. */
+   if (port_tuple_vtab.dump_msgpack(port, out) < 0)
      goto err;
-   }
  } else {
    int keys = 1;
-   assert(port_tuple->size == 0);
+   assert(((struct port_tuple *)port)->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;
@@ -643,7 +672,66 @@ err:
    }
  }
 finish:
- port_destroy(&response->port);
- sqlite3_finalize(stmt);
+ port_destroy(port);
  return rc;
 }
+
+/**
+ * Execute prepared SQL statement.
+ *
+ * This function uses region to allocate memory for temporary
+ * objects. After this function, region will be in the same state
+ * in which it was before this function.
+ *
+ * @param db SQL handle.
+ * @param stmt Prepared statement.
+ * @param[out] port Port to store SQL response.
+ * @param region Region to allocate temporary objects.
+ *
+ * @retval  0 Success.
+ * @retval -1 Error.
+ */
+static inline int
+sql_execute(sqlite3 *db, struct sqlite3_stmt *stmt, struct port *port,
+     struct region *region)
+{
+ int rc, column_count = sqlite3_column_count(stmt);
+ if (column_count > 0) {
+   /* Either ROW or DONE or ERROR. */
+   while ((rc = sqlite3_step(stmt)) == SQLITE_ROW) {
+     if (sql_row_to_port(stmt, column_count, region,
+             port) != 0)
+       return -1;
+   }
+   assert(rc == SQLITE_DONE || rc != SQLITE_OK);
+ } else {
+   /* No rows. Either DONE or ERROR. */
+   rc = sqlite3_step(stmt);
+   assert(rc != SQLITE_ROW && rc != SQLITE_OK);
+ }
+ if (rc != SQLITE_DONE) {
+   diag_set(ClientError, ER_SQL_EXECUTE, sqlite3_errmsg(db));
+   return -1;
+ }
+ return 0;
+}
+
+int
+sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind,
+     uint32_t bind_count, struct port *port,
+     struct region *region)
+{
+ struct sqlite3_stmt *stmt;
+ sqlite3 *db = sql_get();
+ if (sqlite3_prepare_v2(db, sql, len, &stmt, NULL) != SQLITE_OK) {
+   diag_set(ClientError, ER_SQL_EXECUTE, sqlite3_errmsg(db));
+   return -1;
+ }
+ assert(stmt != NULL);
+ port_sql_create(port, stmt);
+ if (sql_bind(stmt, bind, bind_count) == 0 &&
+     sql_execute(db, stmt, port, region) == 0)
+   return 0;
+ port_destroy(port);
+ return -1;
+}
diff --git a/src/box/execute.h b/src/box/execute.h
index 60b8f31..1ed9ab5 100644
--- a/src/box/execute.h
+++ b/src/box/execute.h
@@ -48,18 +48,9 @@ enum sql_info_key {
 
 extern const char *sql_info_key_strs[];
 
-struct obuf;
 struct region;
 struct sql_bind;
 
-/** Response on EXECUTE request. */
-struct sql_response {
- /** Port with response data if any. */
- struct port port;
- /** Prepared SQL statement with metadata. */
- void *prep_stmt;
-};
-
 /**
  * Parse MessagePack array of SQL parameters.
  * @param data MessagePack array of parameters. Each parameter
@@ -78,48 +69,12 @@ int
 sql_bind_list_decode(const char *data, struct sql_bind **out_bind);
 
 /**
- * Dump a built response into @an out buffer. The response is
- * destroyed.
- * Response structure:
- * +----------------------------------------------+
- * | IPROTO_OK, sync, schema_version   ...        | iproto_header
- * +----------------------------------------------+---------------
- * | Body - a map with one or two keys.           |
- * |                                              |
- * | IPROTO_BODY: {                               |
- * |     IPROTO_METADATA: [                       |
- * |         {IPROTO_FIELD_NAME: column name1},   |
- * |         {IPROTO_FIELD_NAME: column name2},   | iproto_body
- * |         ...                                  |
- * |     ],                                       |
- * |                                              |
- * |     IPROTO_DATA: [                           |
- * |         tuple, tuple, tuple, ...             |
- * |     ]                                        |
- * | }                                            |
- * +-------------------- OR ----------------------+
- * | IPROTO_BODY: {                               |
- * |     IPROTO_SQL_INFO: {                       |
- * |         SQL_INFO_ROW_COUNT: number           |
- * |     }                                        |
- * | }                                            |
- * +----------------------------------------------+
- * @param response EXECUTE response.
- * @param out Output buffer.
- *
- * @retval  0 Success.
- * @retval -1 Memory error.
- */
-int
-sql_response_dump(struct sql_response *response, struct obuf *out);
-
-/**
  * Prepare and execute an SQL statement.
  * @param sql SQL statement.
  * @param len Length of @a sql.
  * @param bind Array of parameters.
  * @param bind_count Length of @a bind.
- * @param[out] response Response to store result.
+ * @param[out] port Port to store SQL response.
  * @param region Runtime allocator for temporary objects
  *        (columns, tuples ...).
  *
@@ -128,7 +83,7 @@ sql_response_dump(struct sql_response *response, struct obuf *out);
  */
 int
 sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind,
-     uint32_t bind_count, struct sql_response *response,
+     uint32_t bind_count, struct port *port,
      struct region *region);
 
 #if defined(__cplusplus)
diff --git a/src/box/iproto.cc b/src/box/iproto.cc
index a08c8c5..09e8914 100644
--- a/src/box/iproto.cc
+++ b/src/box/iproto.cc
@@ -1616,7 +1616,7 @@ tx_process_sql(struct cmsg *m)
 {
  struct iproto_msg *msg = tx_accept_msg(m);
  struct obuf *out;
- struct sql_response response;
+ struct port port;
  struct sql_bind *bind;
  int bind_count;
  const char *sql;
@@ -1633,7 +1633,7 @@ tx_process_sql(struct cmsg *m)
    goto error;
  sql = msg->sql.sql_text;
  sql = mp_decode_str(&sql, &len);
- if (sql_prepare_and_execute(sql, len, bind, bind_count, &response,
+ if (sql_prepare_and_execute(sql, len, bind, bind_count, &port,
            &fiber()->gc) != 0)
    goto error;
  /*
@@ -1643,9 +1643,11 @@ tx_process_sql(struct cmsg *m)
  out = msg->connection->tx.p_obuf;
  struct obuf_svp header_svp;
  /* Prepare memory for the iproto header. */
- if (iproto_prepare_header(out, &header_svp, IPROTO_HEADER_LEN) != 0)
+ if (iproto_prepare_header(out, &header_svp, IPROTO_HEADER_LEN) != 0) {
+   port_destroy(&port);
    goto error;
- if (sql_response_dump(&response, out) != 0) {
+ }
+ if (port_dump_msgpack(&port, out) != 0) {
    obuf_rollback_to_svp(out, &header_svp);
    goto error;
  }
diff --git a/src/box/port.h b/src/box/port.h
index ad1b349..f188036 100644
--- a/src/box/port.h
+++ b/src/box/port.h
@@ -65,7 +65,6 @@ extern const struct port_vtab port_tuple_vtab;
 static inline struct port_tuple *
 port_tuple(struct port *port)
 {
- assert(port->vtab == &port_tuple_vtab);
  return (struct port_tuple *)port;
 }

-- 
2.7.4