[tarantool-patches] Re: [PATCH v5 1/1] sql: return all generated ids via IPROTO

[Imeev Mergen <imeevma@tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 24899 bytes --]

Hello! Thank you for review! New patch below.


On 09/29/2018 07:36 AM, Konstantin Osipov wrote:
> *imeevma@tarantool.org  <imeevma@tarantool.org>  [18/09/28 19:53]:
>> diff --git a/src/box/execute.c b/src/box/execute.c
>> index 24459b4..d9fe33f 100644
>> --- a/src/box/execute.c
>> +++ b/src/box/execute.c
>> @@ -42,6 +42,7 @@
>>   #include "schema.h"
>>   #include "port.h"
>>   #include "tuple.h"
>> +#include "sql/vdbeInt.h"
> Ugh. You're including a header which ends with Int. Have you
> thought what this Int means? It's not Integer.
Fixed.
>>   
>>   const char *sql_type_strs[] = {
>>   	NULL,
>> @@ -639,14 +640,37 @@ err:
>>   		assert(port_tuple->size == 0);
>>   		if (iproto_reply_map_key(out, 1, IPROTO_SQL_INFO) != 0)
>>   			goto err;
>> +		struct stailq *id_list = &((struct Vdbe *)stmt)->id_list;
>> +		uint64_t id_count = 0;
> The name is very general. autoinc_id_list would be more specific.
> Why not simply call it generated_ids here and everywhere? This is
> what these values are called in the standard.
Fixed.
>>   		int changes = sqlite3_changes(db);
>>   		int size = mp_sizeof_uint(SQL_INFO_ROW_COUNT) +
>>   			   mp_sizeof_uint(changes);
>> +		if (stailq_empty(id_list) == 0) {
> What is the point of this == 0? Why are you comparing with an
> integer here, not boolean?
Fixed.
>> +			struct id_entry *id_entry;
>> +			stailq_foreach_entry(id_entry, id_list, link) {
>> +				size += id_entry->id >= 0 ?
>> +					mp_sizeof_uint(id_entry->id) :
>> +					mp_sizeof_int(id_entry->id);
> How can you get a negative id?
It is possible:

box.sql.execute('create table a (id int primary key autoincrement)')
box.schema.sequence.alter('A', {min=-10000, step=-1})
box.sql.execute('insert into a values (null)')
box.sql.execute('insert into a values (null)')
box.sql.execute('insert into a values (null)')
box.sql.execute('insert into a values (null)')
box.sql.execute('insert into a values (null)')
box.sql.execute('select * from a')
---
- - [-3]
   - [-2]
   - [-1]
   - [0]
   - [1]
...

>> +				id_count++;
>> +			}
>> +			size += mp_sizeof_uint(SQL_INFO_GENERATED_IDS) +
>> +				mp_sizeof_array(id_count);
>> +		}
>>   		char *buf = obuf_alloc(out, size);
>>   		if (buf == NULL) {
>>   			diag_set(OutOfMemory, size, "obuf_alloc", "buf");
>>   			goto err;
>>   		}
>> +		if (id_count > 0) {
>> +			buf = mp_encode_uint(buf, SQL_INFO_GENERATED_IDS);
>> +			buf = mp_encode_array(buf, id_count);
>> +			struct id_entry *id_entry;
>> +			stailq_foreach_entry(id_entry, id_list, link) {
>> +				buf = id_entry->id >= 0 ?
>> +				      mp_encode_uint(buf, id_entry->id) :
>> +				      mp_encode_int(buf, id_entry->id);
>> +			}
>> +		}
>>   		buf = mp_encode_uint(buf, SQL_INFO_ROW_COUNT);
>>   		buf = mp_encode_uint(buf, changes);
>>   	}
>> diff --git a/src/box/execute.h b/src/box/execute.h
>> index f21393b..614d3d0 100644
>> --- a/src/box/execute.h
>> +++ b/src/box/execute.h
>> @@ -42,6 +42,7 @@ extern "C" {
>>   /** Keys of IPROTO_SQL_INFO map. */
>>   enum sql_info_key {
>>   	SQL_INFO_ROW_COUNT = 0,
>> +	SQL_INFO_GENERATED_IDS = 1,
>>   	sql_info_key_MAX,
>>   };
>>   
>> diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
>> index a928a4c..5d2ac78 100644
>> --- a/src/box/lua/net_box.c
>> +++ b/src/box/lua/net_box.c
>> @@ -671,11 +671,28 @@ netbox_decode_sql_info(struct lua_State *L, const char **data)
>>   	/* Only SQL_INFO_ROW_COUNT is available. */
>>   	assert(map_size == 1);
>>   	(void) map_size;
>> +	lua_newtable(L);
>>   	uint32_t key = mp_decode_uint(data);
>> +	/*
>> +	 * If SQL_INFO_GENERATED_IDS in data then it should be
>> +	 * just before SQL_INFO_ROW_COUNT.
>> +	 */
>> +	if (key == SQL_INFO_GENERATED_IDS) {
>> +		uint64_t count = mp_decode_array(data);
>> +		assert (count > 0);
>> +		lua_createtable(L, 0, count);
>> +		lua_setfield(L, -2, "generated_ids");
>> +		lua_getfield(L, -1, "generated_ids");
>> +		for (uint32_t j = 0; j < count; ++j) {
>> +			int64_t value = mp_decode_uint(data);
>> +			lua_pushinteger(L, value);
>> +			lua_rawseti(L, -2, j + 1);
>> +		}
>> +		lua_pop(L, 1);
>> +		key = mp_decode_uint(data);
>> +	}
>>   	assert(key == SQL_INFO_ROW_COUNT);
>> -	(void) key;
>>   	uint32_t row_count = mp_decode_uint(data);
>> -	lua_createtable(L, 0, 1);
>>   	lua_pushinteger(L, row_count);
>>   	lua_setfield(L, -2, "rowcount");
>>   }
>> diff --git a/src/box/sequence.c b/src/box/sequence.c
>> index 35b7605..ed6b2da 100644
>> --- a/src/box/sequence.c
>> +++ b/src/box/sequence.c
>> @@ -38,6 +38,7 @@
>>   #include <small/mempool.h>
>>   #include <msgpuck/msgpuck.h>
>>   
>> +#include "txn.h"
>>   #include "diag.h"
>>   #include "error.h"
>>   #include "errcode.h"
>> @@ -178,6 +179,30 @@ sequence_update(struct sequence *seq, int64_t value)
>>   	return 0;
>>   }
>>   
>> +/**
>> + * Save generated id in VDBE.
>> + *
>> + * @param value ID to save in VDBE.
>> + * @retval 0 Success.
>> + * @retval -1 Error.
>> + */
>> +static inline int
>> +add_new_id_in_vdbe(int64_t value)
> subject-verb-object
Fixed.
>> +{
>> +	struct txn *txn = in_txn();
>> +	if (txn == NULL || txn->psql_txn == NULL)
>> +		return 0;
>> +	assert(txn->psql_txn->vdbe != NULL);
>> +	struct id_entry *id_entry = malloc(sizeof(*id_entry));
> Why malloc?
Fixed. To save ids in region fiber_gc() was moved from
txn_commit() to sqlite3VdbeDelete().
>> +	if (id_entry == NULL) {
>> +		diag_set(OutOfMemory, sizeof(*id_entry), "malloc", "id_entry");
>> +		return -1;
>> +	}
>> +	id_entry->id = value;
>> +	stailq_add_tail_entry(txn->psql_txn->id_list, id_entry, link);
>> +	return 0;
>> +}
>> +
>>   int
>>   sequence_next(struct sequence *seq, int64_t *result)
>>   {
>> @@ -194,6 +219,8 @@ sequence_next(struct sequence *seq, int64_t *result)
>>   					  new_data) == light_sequence_end)
>>   			return -1;
>>   		*result = def->start;
>> +		if(add_new_id_in_vdbe(*result) != 0)
>> +			return -1;
>>   		return 0;
>>   	}
>>   	old_data = light_sequence_get(&sequence_data_index, pos);
>> @@ -228,6 +255,8 @@ done:
>>   				   new_data, &old_data) == light_sequence_end)
>>   		unreachable();
>>   	*result = value;
>> +	if(add_new_id_in_vdbe(*result) != 0)
>> +		return -1;
> You continue to miss a space after if.
Fixed.
>>   	return 0;
>>   overflow:
>>   	if (!def->cycle) {
>> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
>> index 00ffb0c..cbec9a2 100644
>> --- a/src/box/sql/vdbe.c
>> +++ b/src/box/sql/vdbe.c
>> @@ -3040,7 +3040,7 @@ case OP_TransactionRollback: {
>>    */
>>   case OP_TTransaction: {
>>   	if (!box_txn()) {
>> -		if (box_txn_begin() != 0) {
>> +		if (sql_txn_begin(p) != 0) {
>>   			rc = SQL_TARANTOOL_ERROR;
>>   			goto abort_due_to_error;
>>   		}
>> diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h
>> index d5da5d5..9b94183 100644
>> --- a/src/box/sql/vdbe.h
>> +++ b/src/box/sql/vdbe.h
>> @@ -179,10 +179,11 @@ Vdbe *sqlite3VdbeCreate(Parse *);
>>    * Allocate and initialize SQL-specific struct which completes
>>    * original Tarantool's txn struct using region allocator.
>>    *
>> + * @param vdbe VDBE that is being prepared or executed.
>>    * @retval NULL on OOM, new psql_txn struct on success.
>>    **/
>>   struct sql_txn *
>> -sql_alloc_txn();
>> +sql_alloc_txn(struct Vdbe *vdbe);
>>   
>>   /**
>>    * Prepare given VDBE to execution: initialize structs connected
>> diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
>> index ce97f49..ca1794c 100644
>> --- a/src/box/sql/vdbeInt.h
>> +++ b/src/box/sql/vdbeInt.h
>> @@ -368,6 +368,9 @@ struct Vdbe {
>>   	/** The auto-commit flag. */
>>   	bool auto_commit;
>>   
>> +	/** List of id generated in current VDBE. */
>> +	struct stailq id_list;
>> +
>>   	/* When allocating a new Vdbe object, all of the fields below should be
>>   	 * initialized to zero or NULL
>>   	 */
>> diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
>> index 54edf1b..98ddef7 100644
>> --- a/src/box/sql/vdbeaux.c
>> +++ b/src/box/sql/vdbeaux.c
>> @@ -57,6 +57,8 @@ sqlite3VdbeCreate(Parse * pParse)
>>   		return 0;
>>   	memset(p, 0, sizeof(Vdbe));
>>   	p->db = db;
>> +	/* Init list of generated ids. */
>> +	stailq_create(&p->id_list);
>>   	if (db->pVdbe) {
>>   		db->pVdbe->pPrev = p;
>>   	}
>> @@ -75,7 +77,7 @@ sqlite3VdbeCreate(Parse * pParse)
>>   }
>>   
>>   struct sql_txn *
>> -sql_alloc_txn()
>> +sql_alloc_txn(struct Vdbe *vdbe)
>>   {
>>   	struct sql_txn *txn = region_alloc_object(&fiber()->gc,
>>   						  struct sql_txn);
>> @@ -86,6 +88,8 @@ sql_alloc_txn()
>>   	}
>>   	txn->pSavepoint = NULL;
>>   	txn->fk_deferred_count = 0;
>> +	txn->vdbe = vdbe;
>> +	txn->id_list = &vdbe->id_list;
> Why do you need both?
Fixed.
>>   	return txn;
>>   }
>>   
>> @@ -103,7 +107,7 @@ sql_vdbe_prepare(struct Vdbe *vdbe)
>>   		 * check FK violations, at least now.
>>   		 */
>>   		if (txn->psql_txn == NULL) {
>> -			txn->psql_txn = sql_alloc_txn();
>> +			txn->psql_txn = sql_alloc_txn(vdbe);
>>   			if (txn->psql_txn == NULL)
>>   				return -1;
>>   		}
>> @@ -2261,7 +2265,7 @@ sql_txn_begin(Vdbe *p)
>>   	struct txn *ptxn = txn_begin(false);
>>   	if (ptxn == NULL)
>>   		return -1;
>> -	ptxn->psql_txn = sql_alloc_txn();
>> +	ptxn->psql_txn = sql_alloc_txn(p);
>>   	if (ptxn->psql_txn == NULL) {
>>   		box_txn_rollback();
>>   		return -1;
>> @@ -2746,6 +2750,11 @@ sqlite3VdbeClearObject(sqlite3 * db, Vdbe * p)
>>   	vdbeFreeOpArray(db, p->aOp, p->nOp);
>>   	sqlite3DbFree(db, p->aColName);
>>   	sqlite3DbFree(db, p->zSql);
>> +	while (stailq_empty(&p->id_list) == 0) {
>> +		struct id_entry *id_entry =
>> +			stailq_shift_entry(&p->id_list, struct id_entry, link);
>> +		free(id_entry);
>> +	}
>>   #ifdef SQLITE_ENABLE_STMT_SCANSTATUS
>>   	{

*New patch:*

commit 9dd5e9833aec54c819e6dae07d62624538f76246
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Wed Sep 26 22:07:54 2018 +0300

     sql: return all generated ids via IPROTO

     According to documentation some JDBC functions have an ability to
     return all ids that were generated in executed INSERT statement.
     This patch gives a way to implement such functionality.

     Closes #2618

diff --git a/src/box/execute.c b/src/box/execute.c
index 24459b4..ebf3962 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -42,6 +42,7 @@
  #include "schema.h"
  #include "port.h"
  #include "tuple.h"
+#include "sql/vdbe.h"

  const char *sql_type_strs[] = {
      NULL,
@@ -637,11 +638,26 @@ err:
      } else {
          keys = 1;
          assert(port_tuple->size == 0);
-        if (iproto_reply_map_key(out, 1, IPROTO_SQL_INFO) != 0)
+        struct stailq *generated_ids =
+            vdbe_generated_ids((struct Vdbe *)stmt);
+        uint32_t map_size = stailq_empty(generated_ids) ? 1 : 2;
+        if (iproto_reply_map_key(out, map_size, IPROTO_SQL_INFO) != 0)
              goto err;
+        uint64_t id_count = 0;
          int changes = sqlite3_changes(db);
          int size = mp_sizeof_uint(SQL_INFO_ROW_COUNT) +
                 mp_sizeof_uint(changes);
+        if (!stailq_empty(generated_ids)) {
+            struct id_entry *id_entry;
+            stailq_foreach_entry(id_entry, generated_ids, link) {
+                size += id_entry->id >= 0 ?
+                    mp_sizeof_uint(id_entry->id) :
+                    mp_sizeof_int(id_entry->id);
+                id_count++;
+            }
+            size += mp_sizeof_uint(SQL_INFO_GENERATED_IDS) +
+                mp_sizeof_array(id_count);
+        }
          char *buf = obuf_alloc(out, size);
          if (buf == NULL) {
              diag_set(OutOfMemory, size, "obuf_alloc", "buf");
@@ -649,6 +665,16 @@ err:
          }
          buf = mp_encode_uint(buf, SQL_INFO_ROW_COUNT);
          buf = mp_encode_uint(buf, changes);
+        if (!stailq_empty(generated_ids)) {
+            buf = mp_encode_uint(buf, SQL_INFO_GENERATED_IDS);
+            buf = mp_encode_array(buf, id_count);
+            struct id_entry *id_entry;
+            stailq_foreach_entry(id_entry, generated_ids, link) {
+                buf = id_entry->id >= 0 ?
+                      mp_encode_uint(buf, id_entry->id) :
+                      mp_encode_int(buf, id_entry->id);
+            }
+        }
      }
      iproto_reply_sql(out, &header_svp, response->sync, schema_version,
               keys);
diff --git a/src/box/execute.h b/src/box/execute.h
index f21393b..614d3d0 100644
--- a/src/box/execute.h
+++ b/src/box/execute.h
@@ -42,6 +42,7 @@ extern "C" {
  /** Keys of IPROTO_SQL_INFO map. */
  enum sql_info_key {
      SQL_INFO_ROW_COUNT = 0,
+    SQL_INFO_GENERATED_IDS = 1,
      sql_info_key_MAX,
  };

diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
index a928a4c..1a9b3f7 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -668,16 +668,37 @@ static void
  netbox_decode_sql_info(struct lua_State *L, const char **data)
  {
      uint32_t map_size = mp_decode_map(data);
-    /* Only SQL_INFO_ROW_COUNT is available. */
-    assert(map_size == 1);
-    (void) map_size;
+    assert(map_size == 1 || map_size == 2);
+    lua_newtable(L);
+    /*
+     * First element in data is SQL_INFO_ROW_COUNT.
+     */
      uint32_t key = mp_decode_uint(data);
      assert(key == SQL_INFO_ROW_COUNT);
-    (void) key;
      uint32_t row_count = mp_decode_uint(data);
-    lua_createtable(L, 0, 1);
      lua_pushinteger(L, row_count);
      lua_setfield(L, -2, "rowcount");
+    /*
+     * If data have two elements then second is
+     * SQL_INFO_GENERATED_IDS.
+     */
+    if (map_size == 2) {
+        key = mp_decode_uint(data);
+        assert(key == SQL_INFO_GENERATED_IDS);
+        (void)key;
+        uint64_t count = mp_decode_array(data);
+        assert (count > 0);
+        lua_createtable(L, 0, count);
+        lua_setfield(L, -2, "generated_ids");
+        lua_getfield(L, -1, "generated_ids");
+        for (uint32_t j = 0; j < count; ++j) {
+            int64_t id;
+            mp_read_int64(data, &id);
+            lua_pushinteger(L, id);
+            lua_rawseti(L, -2, j + 1);
+        }
+        lua_pop(L, 1);
+    }
  }

  static int
diff --git a/src/box/sequence.c b/src/box/sequence.c
index 35b7605..b1c68e9 100644
--- a/src/box/sequence.c
+++ b/src/box/sequence.c
@@ -38,6 +38,7 @@
  #include <small/mempool.h>
  #include <msgpuck/msgpuck.h>

+#include "txn.h"
  #include "diag.h"
  #include "error.h"
  #include "errcode.h"
@@ -194,6 +195,9 @@ sequence_next(struct sequence *seq, int64_t *result)
                        new_data) == light_sequence_end)
              return -1;
          *result = def->start;
+        if (txn_vdbe() != NULL &&
+            vdbe_add_new_generated_id(txn_vdbe(), *result) != 0)
+            return -1;
          return 0;
      }
      old_data = light_sequence_get(&sequence_data_index, pos);
@@ -228,6 +232,9 @@ done:
                     new_data, &old_data) == light_sequence_end)
          unreachable();
      *result = value;
+    if (txn_vdbe() != NULL &&
+        vdbe_add_new_generated_id(txn_vdbe(), *result) != 0)
+        return -1;
      return 0;
  overflow:
      if (!def->cycle) {
diff --git a/src/box/sequence.h b/src/box/sequence.h
index 0f6c8da..d38fb2f 100644
--- a/src/box/sequence.h
+++ b/src/box/sequence.h
@@ -43,6 +43,7 @@ extern "C" {
  #endif /* defined(__cplusplus) */

  struct iterator;
+struct Vdbe;

  /** Sequence metadata. */
  struct sequence_def {
@@ -140,6 +141,17 @@ int
  access_check_sequence(struct sequence *seq);

  /**
+ * Add new generated id in VDBE.
+ *
+ * @param Vdbe VDBE to save id in.
+ * @param id ID to save in VDBE.
+ * @retval 0 Success.
+ * @retval -1 Error.
+ */
+int
+vdbe_add_new_generated_id(struct Vdbe *vdbe, int64_t id);
+
+/**
   * Create an iterator over sequence data.
   *
   * The iterator creates a snapshot of sequence data and walks
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 7c1015c..7a3748e 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -578,6 +578,26 @@ out2Prerelease(Vdbe *p, VdbeOp *pOp)
      }
  }

+struct stailq *
+vdbe_generated_ids(struct Vdbe *vdbe)
+{
+    return &vdbe->generated_ids;
+}
+
+int
+vdbe_add_new_generated_id(struct Vdbe *vdbe, int64_t id)
+{
+    assert(vdbe != NULL);
+    struct id_entry *id_entry = region_alloc(&fiber()->gc, 
sizeof(*id_entry));
+    if (id_entry == NULL) {
+        diag_set(OutOfMemory, sizeof(*id_entry), "region_alloc", 
"id_entry");
+        return -1;
+    }
+    id_entry->id = id;
+    stailq_add_tail_entry(vdbe_generated_ids(vdbe), id_entry, link);
+    return 0;
+}
+
  /*
   * Execute as much of a VDBE program as we can.
   * This is the core of sqlite3_step().
@@ -2996,7 +3016,7 @@ case OP_TransactionBegin: {
   */
  case OP_TransactionCommit: {
      if (box_txn()) {
-        if (box_txn_commit() != 0) {
+        if (txn_commit(in_txn()) != 0) {
              rc = SQL_TARANTOOL_ERROR;
              goto abort_due_to_error;
          }
@@ -3040,7 +3060,7 @@ case OP_TransactionRollback: {
   */
  case OP_TTransaction: {
      if (!box_txn()) {
-        if (box_txn_begin() != 0) {
+        if (sql_txn_begin(p) != 0) {
              rc = SQL_TARANTOOL_ERROR;
              goto abort_due_to_error;
          }
diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h
index d5da5d5..8f877b3 100644
--- a/src/box/sql/vdbe.h
+++ b/src/box/sql/vdbe.h
@@ -197,6 +197,15 @@ sql_alloc_txn();
  int
  sql_vdbe_prepare(struct Vdbe *vdbe);

+/**
+ * Return pointer to list of generated ids.
+ *
+ * @param vdbe VDBE to get list of generated ids from.
+ * @retval List of generated ids.
+ */
+struct stailq *
+vdbe_generated_ids(struct Vdbe *vdbe);
+
  int sqlite3VdbeAddOp0(Vdbe *, int);
  int sqlite3VdbeAddOp1(Vdbe *, int, int);
  int sqlite3VdbeAddOp2(Vdbe *, int, int, int);
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index ce97f49..37df5e2 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -368,6 +368,9 @@ struct Vdbe {
      /** The auto-commit flag. */
      bool auto_commit;

+    /** List of id generated in current VDBE. */
+    struct stailq generated_ids;
+
      /* When allocating a new Vdbe object, all of the fields below 
should be
       * initialized to zero or NULL
       */
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index 6e42d05..0023dc6 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -57,6 +57,8 @@ sqlite3VdbeCreate(Parse * pParse)
          return 0;
      memset(p, 0, sizeof(Vdbe));
      p->db = db;
+    /* Init list of generated ids. */
+    stailq_create(&p->generated_ids);
      if (db->pVdbe) {
          db->pVdbe->pPrev = p;
      }
@@ -106,6 +108,7 @@ sql_vdbe_prepare(struct Vdbe *vdbe)
              txn->psql_txn = sql_alloc_txn();
              if (txn->psql_txn == NULL)
                  return -1;
+            txn->psql_txn->vdbe = vdbe;
          }
          vdbe->psql_txn = txn->psql_txn;
      } else {
@@ -2266,6 +2269,7 @@ sql_txn_begin(Vdbe *p)
          box_txn_rollback();
          return -1;
      }
+    ptxn->psql_txn->vdbe = p;
      p->psql_txn = ptxn->psql_txn;
      return 0;
  }
@@ -2414,9 +2418,9 @@ sqlite3VdbeHalt(Vdbe * p)
                       * key constraints to hold up the transaction. 
This means a commit
                       * is required.
                       */
-                    rc = box_txn_commit() ==
-                            0 ? SQLITE_OK :
-                            SQL_TARANTOOL_ERROR;
+                    rc = (in_txn() == NULL ||
+                          txn_commit(in_txn()) == 0) ?
+                         SQLITE_OK : SQL_TARANTOOL_ERROR;
                      closeCursorsAndFree(p);
                  }
                  if (rc == SQLITE_BUSY && !p->pDelFrame) {
@@ -2780,6 +2784,8 @@ sqlite3VdbeDelete(Vdbe * p)
      }
      p->magic = VDBE_MAGIC_DEAD;
      p->db = 0;
+    if (in_txn() == NULL)
+        fiber_gc();
      sqlite3DbFree(db, p);
  }

diff --git a/src/box/txn.h b/src/box/txn.h
index e74c14d..2acba9f 100644
--- a/src/box/txn.h
+++ b/src/box/txn.h
@@ -117,6 +117,17 @@ struct sql_txn {
       * VDBE to the next in the same transaction.
       */
      uint32_t fk_deferred_count;
+    /** Current VDBE. */
+    struct Vdbe *vdbe;
+};
+
+/** An element of list of generated ids. */
+struct id_entry
+{
+    /** A link in a generated id list. */
+    struct stailq_entry link;
+    /** Generated id. */
+    int64_t id;
  };

  struct txn {
@@ -337,6 +348,20 @@ txn_last_stmt(struct txn *txn)
  void
  txn_on_stop(struct trigger *trigger, void *event);

+/**
+ * Return VDBE that is currently executed.
+ *
+ * @retval VDBE that is currently executed.
+ * @retval NULL Either txn or ptxn_sql or vdbe is NULL;
+ */
+static inline struct Vdbe *
+txn_vdbe()
+{
+    struct txn *txn = in_txn();
+    if (txn == NULL || txn->psql_txn == NULL)
+        return NULL;
+    return txn->psql_txn->vdbe;
+}

  /**
   * FFI bindings: do not throw exceptions, do not accept extra
diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index af474bc..163fb6e 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -580,6 +580,77 @@ cn:close()
  box.sql.execute('drop table test')
  ---
  ...
+-- gh-2618 Return generated columns after INSERT in IPROTO.
+-- Return all ids generated in current INSERT statement.
+box.sql.execute('create table test (id integer primary key 
autoincrement, a integer)')
+---
+...
+cn = remote.connect(box.cfg.listen)
+---
+...
+cn:execute('insert into test values (1, 1)')
+---
+- rowcount: 1
+...
+cn:execute('insert into test values (null, 2)')
+---
+- generated_ids:
+  - 2
+  rowcount: 1
+...
+cn:execute('update test set a = 11 where id == 1')
+---
+- rowcount: 1
+...
+cn:execute('insert into test values (100, 1), (null, 1), (120, 1), 
(null, 1)')
+---
+- generated_ids:
+  - 101
+  - 121
+  rowcount: 4
+...
+cn:execute('insert into test values (null, 1), (null, 1), (null, 1), 
(null, 1), (null, 1)')
+---
+- generated_ids:
+  - 122
+  - 123
+  - 124
+  - 125
+  - 126
+  rowcount: 5
+...
+cn:execute('select * from test')
+---
+- metadata:
+  - name: ID
+  - name: A
+  rows:
+  - [1, 11]
+  - [2, 2]
+  - [100, 1]
+  - [101, 1]
+  - [120, 1]
+  - [121, 1]
+  - [122, 1]
+  - [123, 1]
+  - [124, 1]
+  - [125, 1]
+  - [126, 1]
+...
+cn:execute('create table test2 (id integer primary key, a integer)')
+---
+- rowcount: 1
+...
+cn:execute('drop table test2')
+---
+- rowcount: 1
+...
+cn:close()
+---
+...
+box.sql.execute('drop table test')
+---
+...
  box.schema.user.revoke('guest', 'read,write,execute', 'universe')
  ---
  ...
diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua
index 220331b..6517cde 100644
--- a/test/sql/iproto.test.lua
+++ b/test/sql/iproto.test.lua
@@ -201,6 +201,21 @@ future4:wait_result()
  cn:close()
  box.sql.execute('drop table test')

+-- gh-2618 Return generated columns after INSERT in IPROTO.
+-- Return all ids generated in current INSERT statement.
+box.sql.execute('create table test (id integer primary key 
autoincrement, a integer)')
+cn = remote.connect(box.cfg.listen)
+cn:execute('insert into test values (1, 1)')
+cn:execute('insert into test values (null, 2)')
+cn:execute('update test set a = 11 where id == 1')
+cn:execute('insert into test values (100, 1), (null, 1), (120, 1), 
(null, 1)')
+cn:execute('insert into test values (null, 1), (null, 1), (null, 1), 
(null, 1), (null, 1)')
+cn:execute('select * from test')
+cn:execute('create table test2 (id integer primary key, a integer)')
+cn:execute('drop table test2')
+cn:close()
+box.sql.execute('drop table test')
+
  box.schema.user.revoke('guest', 'read,write,execute', 'universe')
  space = nil



[-- Attachment #2: Type: text/html, Size: 30785 bytes --]