[tarantool-patches] [PATCH v7 0/2] sql: return all generated ids via IPROTO

[tarantool-patches] [PATCH v7 0/2] sql: return all generated ids via IPROTO

[imeevma]

To make implementation of function getGeneratedKeys() of JDBC
driver  we should be able to return information of generated keys
through IPROTO.

Patch 1 implements such functionality.

Patch 2 do some refactoring in VDBE.

Issue: https://github.com/tarantool/tarantool/issues/2618
Branch: https://github.com/tarantool/tarantool/tree/imeevma/gh-2618-return-all-generated-ids

Mergen Imeev (1):
  sql: return all generated ids via IPROTO

Vladislav Shpilevoy (1):
  sql: remove psql_txn from Vdbe

 src/box/execute.c        | 28 ++++++++++++++-
 src/box/execute.h        |  1 +
 src/box/lua/net_box.c    | 29 ++++++++++++---
 src/box/sequence.c       |  7 ++++
 src/box/sequence.h       | 13 +++++++
 src/box/sql/vdbe.c       | 42 +++++++++++++++++-----
 src/box/sql/vdbe.h       | 12 ++++++-
 src/box/sql/vdbeInt.h    |  7 ++--
 src/box/sql/vdbeaux.c    | 25 ++++++-------
 src/box/txn.h            | 27 ++++++++++++++
 test/sql/iproto.result   | 92 ++++++++++++++++++++++++++++++++++++++++++++++++
 test/sql/iproto.test.lua | 24 +++++++++++++
 12 files changed, 277 insertions(+), 30 deletions(-)

-- 
2.7.4

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

[imeevma]

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
---
 src/box/execute.c        | 28 ++++++++++++++-
 src/box/execute.h        |  1 +
 src/box/lua/net_box.c    | 29 ++++++++++++---
 src/box/sequence.c       |  7 ++++
 src/box/sequence.h       | 13 +++++++
 src/box/sql/vdbe.c       | 27 ++++++++++++--
 src/box/sql/vdbe.h       | 12 ++++++-
 src/box/sql/vdbeInt.h    |  5 +++
 src/box/sql/vdbeaux.c    | 21 ++++++-----
 src/box/txn.h            | 27 ++++++++++++++
 test/sql/iproto.result   | 92 ++++++++++++++++++++++++++++++++++++++++++++++++
 test/sql/iproto.test.lua | 24 +++++++++++++
 12 files changed, 268 insertions(+), 18 deletions(-)

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..5a137e0 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -668,16 +668,35 @@ 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);
+		for (uint32_t j = 0; j < count; ++j) {
+			int64_t id;
+			mp_read_int64(data, &id);
+			luaL_pushint64(L, id);
+			lua_rawseti(L, -2, j + 1);
+		}
+		lua_setfield(L, -2, "generated_ids");
+	}
 }
 
 static int
diff --git a/src/box/sequence.c b/src/box/sequence.c
index 35b7605..9a87a56 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(), value) != 0)
+		return -1;
 	return 0;
 overflow:
 	if (!def->cycle) {
diff --git a/src/box/sequence.h b/src/box/sequence.h
index 0f6c8da..7180ead 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,18 @@ int
 access_check_sequence(struct sequence *seq);
 
 /**
+ * Append a new sequence value to Vdbe. List of automatically
+ * generated identifiers is returned as metadata of SQL response.
+ *
+ * @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..2571492 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().
@@ -2995,8 +3015,9 @@ case OP_TransactionBegin: {
  * If there is no active transaction, raise an error.
  */
 case OP_TransactionCommit: {
-	if (box_txn()) {
-		if (box_txn_commit() != 0) {
+	struct txn *txn = in_txn();
+	if (txn != NULL) {
+		if (txn_commit(txn) != 0) {
 			rc = SQL_TARANTOOL_ERROR;
 			goto abort_due_to_error;
 		}
@@ -3040,7 +3061,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..ed7fb2f 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 v Vdbe with which associate this transaction.
  * @retval NULL on OOM, new psql_txn struct on success.
  **/
 struct sql_txn *
-sql_alloc_txn();
+sql_alloc_txn(struct Vdbe *v);
 
 /**
  * Prepare given VDBE to execution: initialize structs connected
@@ -197,6 +198,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..7a7d3de 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -367,6 +367,11 @@ struct Vdbe {
 	struct sql_txn *psql_txn;
 	/** The auto-commit flag. */
 	bool auto_commit;
+	/**
+	 * List of ids generated in current VDBE. It is returned
+	 * as metadata of SQL response.
+	 */
+	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..ae73f25 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -57,6 +57,7 @@ sqlite3VdbeCreate(Parse * pParse)
 		return 0;
 	memset(p, 0, sizeof(Vdbe));
 	p->db = db;
+	stailq_create(&p->generated_ids);
 	if (db->pVdbe) {
 		db->pVdbe->pPrev = p;
 	}
@@ -75,7 +76,7 @@ sqlite3VdbeCreate(Parse * pParse)
 }
 
 struct sql_txn *
-sql_alloc_txn()
+sql_alloc_txn(struct Vdbe *v)
 {
 	struct sql_txn *txn = region_alloc_object(&fiber()->gc,
 						  struct sql_txn);
@@ -84,6 +85,8 @@ sql_alloc_txn()
 			 "struct sql_txn");
 		return NULL;
 	}
+	txn->vdbe = v;
+	v->psql_txn = txn;
 	txn->pSavepoint = NULL;
 	txn->fk_deferred_count = 0;
 	return txn;
@@ -103,11 +106,12 @@ 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;
+		} else {
+			vdbe->psql_txn = txn->psql_txn;
 		}
-		vdbe->psql_txn = txn->psql_txn;
 	} else {
 		vdbe->psql_txn = NULL;
 	}
@@ -2261,12 +2265,11 @@ 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;
 	}
-	p->psql_txn = ptxn->psql_txn;
 	return 0;
 }
 
@@ -2414,9 +2417,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 +2783,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..12e9d10 100644
--- a/src/box/txn.h
+++ b/src/box/txn.h
@@ -117,6 +117,19 @@ 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 autogenerated ids, being returned as SQL
+ * response metadata.
+ */
+struct id_entry {
+	/** A link in a generated id list. */
+	struct stailq_entry link;
+	/** Generated id. */
+	int64_t id;
 };
 
 struct txn {
@@ -337,6 +350,20 @@ txn_last_stmt(struct txn *txn)
 void
 txn_on_stop(struct trigger *trigger, void *event);
 
+/**
+ * Return VDBE that is being currently executed.
+ *
+ * @retval VDBE that is being 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..af4fc12 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -580,6 +580,98 @@ 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]
+...
+s = box.schema.create_space('test2', {engine = engine})
+---
+...
+sq = box.schema.sequence.create('test2')
+---
+...
+pk = s:create_index('pk', {sequence = 'test2'})
+---
+...
+function push_id() s:replace{box.NULL} s:replace{box.NULL} end
+---
+...
+_ = box.space.TEST:on_replace(push_id)
+---
+...
+cn:execute('insert into test values (null, 1)')
+---
+- generated_ids:
+  - 127
+  - 1
+  - 2
+  rowcount: 1
+...
+cn:close()
+---
+...
+box.sql.execute('drop table test')
+---
+...
+s:drop()
+---
+...
+sq:drop()
+---
+...
 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..a2caa57 100644
--- a/test/sql/iproto.test.lua
+++ b/test/sql/iproto.test.lua
@@ -201,6 +201,30 @@ 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')
+
+s = box.schema.create_space('test2', {engine = engine})
+sq = box.schema.sequence.create('test2')
+pk = s:create_index('pk', {sequence = 'test2'})
+function push_id() s:replace{box.NULL} s:replace{box.NULL} end
+_ = box.space.TEST:on_replace(push_id)
+cn:execute('insert into test values (null, 1)')
+
+cn:close()
+box.sql.execute('drop table test')
+s:drop()
+sq:drop()
+
 box.schema.user.revoke('guest', 'read,write,execute', 'universe')
 space = nil
 
-- 
2.7.4

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

[Konstantin Osipov]

* imeevma@tarantool.org <imeevma@tarantool.org> [18/10/29 20:25]:
> 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.

Very nice. 

The trick with fiber_gc() is also good IMHO.
Thank you for working on this!

We're going to do a lot of integration of box and vdbe - triggers,
constraints, etc, so I think this patch is on track from architecture
point of view as well, we will need to access vdbe from box increasingly
more as we progress along with the integration.

> Closes #261
-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

[tarantool-patches] [PATCH v7 2/2] sql: remove psql_txn from Vdbe

[imeevma]

It makes no sense to store it here, and it has never
did. SQL transaction specific things shall be taken
from global txn object, as any transaction specific
things.
---
 src/box/sql/vdbe.c    | 15 +++++++++------
 src/box/sql/vdbeInt.h |  2 --
 src/box/sql/vdbeaux.c | 10 +++-------
 3 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 2571492..1e732a9 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2882,7 +2882,8 @@ case OP_Savepoint: {
 	Savepoint *pNew;
 	Savepoint *pSavepoint;
 	Savepoint *pTmp;
-	struct sql_txn *psql_txn = p->psql_txn;
+	struct txn *txn = in_txn();
+	struct sql_txn *psql_txn = txn != NULL ? txn->psql_txn : NULL;
 
 	if (psql_txn == NULL) {
 		assert(!box_txn());
@@ -2960,7 +2961,7 @@ case OP_Savepoint: {
 				assert(pSavepoint == psql_txn->pSavepoint);
 				psql_txn->pSavepoint = pSavepoint->pNext;
 			} else {
-				p->psql_txn->fk_deferred_count =
+				psql_txn->fk_deferred_count =
 					pSavepoint->tnt_savepoint->fk_deferred_count;
 			}
 		}
@@ -4802,8 +4803,9 @@ case OP_Param: {           /* out2 */
 case OP_FkCounter: {
 	if ((user_session->sql_flags & SQLITE_DeferFKs || pOp->p1 != 0) &&
 	    !p->auto_commit) {
-		assert(p->psql_txn != NULL);
-		p->psql_txn->fk_deferred_count += pOp->p2;
+		struct txn *txn = in_txn();
+		assert(txn != NULL && txn->psql_txn != NULL);
+		txn->psql_txn->fk_deferred_count += pOp->p2;
 	} else {
 		p->nFkConstraint += pOp->p2;
 	}
@@ -4825,8 +4827,9 @@ case OP_FkCounter: {
 case OP_FkIfZero: {         /* jump */
 	if ((user_session->sql_flags & SQLITE_DeferFKs || pOp->p1) &&
 	    !p->auto_commit) {
-		assert(p->psql_txn != NULL);
-		if (p->psql_txn->fk_deferred_count == 0)
+		struct txn *txn = in_txn();
+		assert(txn != NULL && txn->psql_txn != NULL);
+		if (txn->psql_txn->fk_deferred_count == 0)
 			goto jump_to_p2;
 	} else {
 		if (p->nFkConstraint == 0)
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index 7a7d3de..19b35b7 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -363,8 +363,6 @@ struct Vdbe {
 	 * ignoreRaised variable helps to track such situations
 	 */
 	u8 ignoreRaised;	/* Flag for ON CONFLICT IGNORE for triggers */
-	/** Data related to current transaction. */
-	struct sql_txn *psql_txn;
 	/** The auto-commit flag. */
 	bool auto_commit;
 	/**
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index ae73f25..3efe252 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -86,7 +86,6 @@ sql_alloc_txn(struct Vdbe *v)
 		return NULL;
 	}
 	txn->vdbe = v;
-	v->psql_txn = txn;
 	txn->pSavepoint = NULL;
 	txn->fk_deferred_count = 0;
 	return txn;
@@ -109,11 +108,7 @@ sql_vdbe_prepare(struct Vdbe *vdbe)
 			txn->psql_txn = sql_alloc_txn(vdbe);
 			if (txn->psql_txn == NULL)
 				return -1;
-		} else {
-			vdbe->psql_txn = txn->psql_txn;
 		}
-	} else {
-		vdbe->psql_txn = NULL;
 	}
 	return 0;
 }
@@ -2244,8 +2239,9 @@ sqlite3VdbeCloseStatement(Vdbe * p, int eOp)
 int
 sqlite3VdbeCheckFk(Vdbe * p, int deferred)
 {
-	if ((deferred && p->psql_txn != NULL &&
-	     p->psql_txn->fk_deferred_count > 0) ||
+	struct txn *txn = in_txn();
+	if ((deferred && txn != NULL && txn->psql_txn != NULL &&
+	     txn->psql_txn->fk_deferred_count > 0) ||
 	    (!deferred && p->nFkConstraint > 0)) {
 		p->rc = SQLITE_CONSTRAINT_FOREIGNKEY;
 		p->errorAction = ON_CONFLICT_ACTION_ABORT;
-- 
2.7.4

[tarantool-patches] Re: [PATCH v7 0/2] sql: return all generated ids via IPROTO

[Vladislav Shpilevoy]

Hi! Nikita, please, ignore this. It contains a bug.

On 27/10/2018 14:18, imeevma@tarantool.org wrote:
> To make implementation of function getGeneratedKeys() of JDBC
> driver  we should be able to return information of generated keys
> through IPROTO.
> 
> Patch 1 implements such functionality.
> 
> Patch 2 do some refactoring in VDBE.
> 
> Issue: https://github.com/tarantool/tarantool/issues/2618
> Branch: https://github.com/tarantool/tarantool/tree/imeevma/gh-2618-return-all-generated-ids
> 
> Mergen Imeev (1):
>    sql: return all generated ids via IPROTO
> 
> Vladislav Shpilevoy (1):
>    sql: remove psql_txn from Vdbe
> 
>   src/box/execute.c        | 28 ++++++++++++++-
>   src/box/execute.h        |  1 +
>   src/box/lua/net_box.c    | 29 ++++++++++++---
>   src/box/sequence.c       |  7 ++++
>   src/box/sequence.h       | 13 +++++++
>   src/box/sql/vdbe.c       | 42 +++++++++++++++++-----
>   src/box/sql/vdbe.h       | 12 ++++++-
>   src/box/sql/vdbeInt.h    |  7 ++--
>   src/box/sql/vdbeaux.c    | 25 ++++++-------
>   src/box/txn.h            | 27 ++++++++++++++
>   test/sql/iproto.result   | 92 ++++++++++++++++++++++++++++++++++++++++++++++++
>   test/sql/iproto.test.lua | 24 +++++++++++++
>   12 files changed, 277 insertions(+), 30 deletions(-)
> 

[tarantool-patches] Re: [PATCH v7 0/2] sql: return all generated ids via IPROTO

[n.pettik]

> On 29 Oct 2018, at 00:28, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> Hi! Nikita, please, ignore this. It contains a bug.

Hello.

NP, will be waiting for updated version.

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

[imeevma]

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
---
 src/box/execute.c        | 28 ++++++++++++++-
 src/box/execute.h        |  1 +
 src/box/lua/net_box.c    | 29 ++++++++++++---
 src/box/sequence.c       |  7 ++++
 src/box/sequence.h       | 13 +++++++
 src/box/sql/vdbe.c       | 27 ++++++++++++--
 src/box/sql/vdbe.h       | 12 ++++++-
 src/box/sql/vdbeInt.h    |  5 +++
 src/box/sql/vdbeaux.c    | 21 ++++++-----
 src/box/txn.c            | 15 +++++---
 src/box/txn.h            | 27 ++++++++++++++
 src/box/vy_scheduler.c   |  1 +
 test/sql/iproto.result   | 92 ++++++++++++++++++++++++++++++++++++++++++++++++
 test/sql/iproto.test.lua | 24 +++++++++++++
 14 files changed, 279 insertions(+), 23 deletions(-)

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..5a137e0 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -668,16 +668,35 @@ 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);
+		for (uint32_t j = 0; j < count; ++j) {
+			int64_t id;
+			mp_read_int64(data, &id);
+			luaL_pushint64(L, id);
+			lua_rawseti(L, -2, j + 1);
+		}
+		lua_setfield(L, -2, "generated_ids");
+	}
 }
 
 static int
diff --git a/src/box/sequence.c b/src/box/sequence.c
index 35b7605..9a87a56 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(), value) != 0)
+		return -1;
 	return 0;
 overflow:
 	if (!def->cycle) {
diff --git a/src/box/sequence.h b/src/box/sequence.h
index 0f6c8da..7180ead 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,18 @@ int
 access_check_sequence(struct sequence *seq);
 
 /**
+ * Append a new sequence value to Vdbe. List of automatically
+ * generated identifiers is returned as metadata of SQL response.
+ *
+ * @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..2571492 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().
@@ -2995,8 +3015,9 @@ case OP_TransactionBegin: {
  * If there is no active transaction, raise an error.
  */
 case OP_TransactionCommit: {
-	if (box_txn()) {
-		if (box_txn_commit() != 0) {
+	struct txn *txn = in_txn();
+	if (txn != NULL) {
+		if (txn_commit(txn) != 0) {
 			rc = SQL_TARANTOOL_ERROR;
 			goto abort_due_to_error;
 		}
@@ -3040,7 +3061,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..ed7fb2f 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 v Vdbe with which associate this transaction.
  * @retval NULL on OOM, new psql_txn struct on success.
  **/
 struct sql_txn *
-sql_alloc_txn();
+sql_alloc_txn(struct Vdbe *v);
 
 /**
  * Prepare given VDBE to execution: initialize structs connected
@@ -197,6 +198,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..7a7d3de 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -367,6 +367,11 @@ struct Vdbe {
 	struct sql_txn *psql_txn;
 	/** The auto-commit flag. */
 	bool auto_commit;
+	/**
+	 * List of ids generated in current VDBE. It is returned
+	 * as metadata of SQL response.
+	 */
+	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..ae73f25 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -57,6 +57,7 @@ sqlite3VdbeCreate(Parse * pParse)
 		return 0;
 	memset(p, 0, sizeof(Vdbe));
 	p->db = db;
+	stailq_create(&p->generated_ids);
 	if (db->pVdbe) {
 		db->pVdbe->pPrev = p;
 	}
@@ -75,7 +76,7 @@ sqlite3VdbeCreate(Parse * pParse)
 }
 
 struct sql_txn *
-sql_alloc_txn()
+sql_alloc_txn(struct Vdbe *v)
 {
 	struct sql_txn *txn = region_alloc_object(&fiber()->gc,
 						  struct sql_txn);
@@ -84,6 +85,8 @@ sql_alloc_txn()
 			 "struct sql_txn");
 		return NULL;
 	}
+	txn->vdbe = v;
+	v->psql_txn = txn;
 	txn->pSavepoint = NULL;
 	txn->fk_deferred_count = 0;
 	return txn;
@@ -103,11 +106,12 @@ 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;
+		} else {
+			vdbe->psql_txn = txn->psql_txn;
 		}
-		vdbe->psql_txn = txn->psql_txn;
 	} else {
 		vdbe->psql_txn = NULL;
 	}
@@ -2261,12 +2265,11 @@ 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;
 	}
-	p->psql_txn = ptxn->psql_txn;
 	return 0;
 }
 
@@ -2414,9 +2417,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 +2783,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.c b/src/box/txn.c
index 617ceb8..103a8aa 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -250,8 +250,12 @@ txn_commit_stmt(struct txn *txn, struct request *request)
 			goto fail;
 	}
 	--txn->in_sub_stmt;
-	if (txn->is_autocommit && txn->in_sub_stmt == 0)
-		return txn_commit(txn);
+	if (txn->is_autocommit && txn->in_sub_stmt == 0) {
+		int rc = txn_commit(txn);
+		if (rc == 0)
+			fiber_gc();
+		return rc;
+	}
 	return 0;
 fail:
 	txn_rollback_stmt();
@@ -354,8 +358,6 @@ txn_commit(struct txn *txn)
 		txn_stmt_unref_tuples(stmt);
 
 	TRASH(txn);
-	/** Free volatile txn memory. */
-	fiber_gc();
 	fiber_set_txn(fiber(), NULL);
 	return 0;
 fail:
@@ -463,7 +465,10 @@ box_txn_commit()
 		diag_set(ClientError, ER_COMMIT_IN_SUB_STMT);
 		return -1;
 	}
-	return txn_commit(txn);
+	int rc = txn_commit(txn);
+	if (rc == 0)
+		fiber_gc();
+	return rc;
 }
 
 int
diff --git a/src/box/txn.h b/src/box/txn.h
index e74c14d..12e9d10 100644
--- a/src/box/txn.h
+++ b/src/box/txn.h
@@ -117,6 +117,19 @@ 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 autogenerated ids, being returned as SQL
+ * response metadata.
+ */
+struct id_entry {
+	/** A link in a generated id list. */
+	struct stailq_entry link;
+	/** Generated id. */
+	int64_t id;
 };
 
 struct txn {
@@ -337,6 +350,20 @@ txn_last_stmt(struct txn *txn)
 void
 txn_on_stop(struct trigger *trigger, void *event);
 
+/**
+ * Return VDBE that is being currently executed.
+ *
+ * @retval VDBE that is being 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/src/box/vy_scheduler.c b/src/box/vy_scheduler.c
index eab3f6c..6e9cc90 100644
--- a/src/box/vy_scheduler.c
+++ b/src/box/vy_scheduler.c
@@ -886,6 +886,7 @@ vy_deferred_delete_batch_process_f(struct cmsg *cmsg)
 
 	if (txn_commit(txn) != 0)
 		goto fail;
+	fiber_gc();
 
 	return;
 fail:
diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index af474bc..af4fc12 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -580,6 +580,98 @@ 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]
+...
+s = box.schema.create_space('test2', {engine = engine})
+---
+...
+sq = box.schema.sequence.create('test2')
+---
+...
+pk = s:create_index('pk', {sequence = 'test2'})
+---
+...
+function push_id() s:replace{box.NULL} s:replace{box.NULL} end
+---
+...
+_ = box.space.TEST:on_replace(push_id)
+---
+...
+cn:execute('insert into test values (null, 1)')
+---
+- generated_ids:
+  - 127
+  - 1
+  - 2
+  rowcount: 1
+...
+cn:close()
+---
+...
+box.sql.execute('drop table test')
+---
+...
+s:drop()
+---
+...
+sq:drop()
+---
+...
 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..a2caa57 100644
--- a/test/sql/iproto.test.lua
+++ b/test/sql/iproto.test.lua
@@ -201,6 +201,30 @@ 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')
+
+s = box.schema.create_space('test2', {engine = engine})
+sq = box.schema.sequence.create('test2')
+pk = s:create_index('pk', {sequence = 'test2'})
+function push_id() s:replace{box.NULL} s:replace{box.NULL} end
+_ = box.space.TEST:on_replace(push_id)
+cn:execute('insert into test values (null, 1)')
+
+cn:close()
+box.sql.execute('drop table test')
+s:drop()
+sq:drop()
+
 box.schema.user.revoke('guest', 'read,write,execute', 'universe')
 space = nil
 
-- 
2.7.4