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

[imeevma@tarantool.org]

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 implements such functinality.

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

 src/box/execute.c        |  17 ++++-
 src/box/execute.h        |   1 +
 src/box/lua/net_box.c    |  16 ++++-
 src/box/sequence.c       |  37 ++++++++++
 src/box/sql/sqliteInt.h  |   4 ++
 src/box/sql/vdbeaux.c    |  29 ++++++++
 src/box/txn.c            |   5 ++
 src/box/txn.h            |   6 ++
 test/sql/errinj.result   |   3 +-
 test/sql/iproto.result   | 171 ++++++++++++++++++++++++++++++++++++++---------
 test/sql/iproto.test.lua |  15 +++++
 11 files changed, 266 insertions(+), 38 deletions(-)

diff --git a/src/box/execute.c b/src/box/execute.c
index 24459b4..8e4bc80 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -641,14 +641,27 @@ err:
 			goto err;
 		int changes = sqlite3_changes(db);
 		int size = mp_sizeof_uint(SQL_INFO_ROW_COUNT) +
-			   mp_sizeof_uint(changes);
+			   mp_sizeof_uint(changes) +
+			   mp_sizeof_uint(SQL_INFO_GENERATED_IDS) +
+			   mp_sizeof_array(db->generated_ids_count);
+		for (uint64_t i = 0; i < db->generated_ids_count; ++i) {
+			size += db->generated_ids_array[i] >= 0 ?
+				mp_sizeof_uint(db->generated_ids_array[i]) :
+				mp_sizeof_int(db->generated_ids_array[i]);
+		}
 		char *buf = obuf_alloc(out, size);
 		if (buf == NULL) {
 			diag_set(OutOfMemory, size, "obuf_alloc", "buf");
-			goto err;
 		}
 		buf = mp_encode_uint(buf, SQL_INFO_ROW_COUNT);
 		buf = mp_encode_uint(buf, changes);
+		buf = mp_encode_uint(buf, SQL_INFO_GENERATED_IDS);
+		buf = mp_encode_array(buf, db->generated_ids_count);
+		for (uint64_t i = 0; i < db->generated_ids_count; ++i) {
+			buf = db->generated_ids_array[i] < 0 ?
+			      mp_encode_int(buf, db->generated_ids_array[i]) :
+			      mp_encode_uint(buf, db->generated_ids_array[i]);
+		}
 	}
 	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..d1dce92 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -668,16 +668,26 @@ 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;
 	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);
+	key = mp_decode_uint(data);
+	assert(key == SQL_INFO_GENERATED_IDS);
+	uint64_t count = mp_decode_array(data);
+	lua_newtable(L);
 	lua_pushinteger(L, row_count);
 	lua_setfield(L, -2, "rowcount");
+	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);
 }
 
 static int
diff --git a/src/box/sequence.c b/src/box/sequence.c
index 35b7605..234509c 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"
@@ -52,6 +53,7 @@
 enum {
 	SEQUENCE_HASH_SEED = 13U,
 	SEQUENCE_DATA_EXTENT_SIZE = 512,
+	SEQUENCE_GENERATED_IDS_MIN_LEN = 16,
 };
 
 /** Sequence state. */
@@ -178,6 +180,37 @@ sequence_update(struct sequence *seq, int64_t value)
 	return 0;
 }
 
+/*
+ * Save generated value into txn.
+ *
+ * \param value value to save.
+ * \retval 0 Success.
+ * \retval -1 Error.
+ */
+static inline int
+save_value(int64_t value)
+{
+	struct txn *txn = in_txn();
+	if (txn == NULL)
+		return 0;
+	if (txn->generated_ids.size == txn->generated_ids.capacity) {
+		txn->generated_ids.capacity = txn->generated_ids.capacity > 0 ?
+					      txn->generated_ids.capacity * 2 :
+					      SEQUENCE_GENERATED_IDS_MIN_LEN;
+		uint64_t capacity = txn->generated_ids.capacity *
+				    sizeof(*txn->generated_ids.array);
+		txn->generated_ids.array = realloc(txn->generated_ids.array,
+						   capacity);
+		if (txn->generated_ids.array == NULL) {
+			diag_set(OutOfMemory, txn->generated_ids.capacity,
+				 "realloc", "txn->generated_ids.array");
+			return -1;
+		}
+	}
+	txn->generated_ids.array[txn->generated_ids.size++] = value;
+	return 0;
+}
+
 int
 sequence_next(struct sequence *seq, int64_t *result)
 {
@@ -194,6 +227,8 @@ sequence_next(struct sequence *seq, int64_t *result)
 					  new_data) == light_sequence_end)
 			return -1;
 		*result = def->start;
+		if(save_value(*result) != 0)
+			return -1;
 		return 0;
 	}
 	old_data = light_sequence_get(&sequence_data_index, pos);
@@ -228,6 +263,8 @@ done:
 				   new_data, &old_data) == light_sequence_end)
 		unreachable();
 	*result = value;
+	if(save_value(*result) != 0)
+		return -1;
 	return 0;
 overflow:
 	if (!def->cycle) {
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 1d32c9a..7d41ece 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -1525,6 +1525,10 @@ struct sqlite3 {
 	u8 mTrace;		/* zero or more SQLITE_TRACE flags */
 	u32 magic;		/* Magic number for detect library misuse */
 	int nChange;		/* Value returned by sqlite3_changes() */
+	/* Number of generated ids. */
+	uint64_t generated_ids_count;
+	/* Array of generated ids. */
+	int64_t *generated_ids_array;
 	int nTotalChange;	/* Value returned by sqlite3_total_changes() */
 	int aLimit[SQLITE_N_LIMIT];	/* Limits */
 	int nMaxSorterMmap;	/* Maximum size of regions mapped by sorter */
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index 3b0c90c..7b5d6b9 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -2303,6 +2303,29 @@ sql_savepoint(Vdbe *p, const char *zName)
 }
 
 /*
+ * Move array of generated ids into db.
+ *
+ * \param db db to save array to.
+ * \retval 0 Success.
+ * \retval -1 Error.
+ */
+static inline int
+move_generated_ids_in_db(sqlite3 *db)
+{
+	struct txn *txn = in_txn();
+	if (txn == NULL)
+		return 0;
+	uint64_t size = txn->generated_ids.size *
+			sizeof(*txn->generated_ids.array);
+	db->generated_ids_count = txn->generated_ids.size;
+	db->generated_ids_array = malloc(size);
+	if (db->generated_ids_array == NULL)
+		return -1;
+	memcpy(db->generated_ids_array, txn->generated_ids.array, size);
+	return 0;
+}
+
+/*
  * This routine is called the when a VDBE tries to halt.  If the VDBE
  * has made changes and is in autocommit mode, then commit those
  * changes.  If a rollback is needed, then do the rollback.
@@ -2420,6 +2443,7 @@ sqlite3VdbeHalt(Vdbe * p)
 					 * key constraints to hold up the transaction. This means a commit
 					 * is required.
 					 */
+					move_generated_ids_in_db(db);
 					rc = box_txn_commit() ==
 						    0 ? SQLITE_OK :
 						    SQL_TARANTOOL_ERROR;
@@ -2751,6 +2775,11 @@ sqlite3VdbeClearObject(sqlite3 * db, Vdbe * p)
 		sqlite3DbFree(db, p->pVList);
 		sqlite3DbFree(db, p->pFree);
 	}
+	/* Free allocated for generated ids array. */
+	db->generated_ids_count = 0;
+	free(db->generated_ids_array);
+	db->generated_ids_array = NULL;
+
 	vdbeFreeOpArray(db, p->aOp, p->nOp);
 	sqlite3DbFree(db, p->aColName);
 	sqlite3DbFree(db, p->zSql);
diff --git a/src/box/txn.c b/src/box/txn.c
index 617ceb8..9af8e4a 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -150,6 +150,9 @@ txn_begin(bool is_autocommit)
 	txn->engine = NULL;
 	txn->engine_tx = NULL;
 	txn->psql_txn = NULL;
+	txn->generated_ids.size = 0;
+	txn->generated_ids.capacity = 0;
+	txn->generated_ids.array = NULL;
 	/* fiber_on_yield/fiber_on_stop initialized by engine on demand */
 	fiber_set_txn(fiber(), txn);
 	return txn;
@@ -353,6 +356,7 @@ txn_commit(struct txn *txn)
 	stailq_foreach_entry(stmt, &txn->stmts, next)
 		txn_stmt_unref_tuples(stmt);
 
+	free(txn->generated_ids.array);
 	TRASH(txn);
 	/** Free volatile txn memory. */
 	fiber_gc();
@@ -395,6 +399,7 @@ txn_rollback()
 	stailq_foreach_entry(stmt, &txn->stmts, next)
 		txn_stmt_unref_tuples(stmt);
 
+	free(txn->generated_ids.array);
 	TRASH(txn);
 	/** Free volatile txn memory. */
 	fiber_gc();
diff --git a/src/box/txn.h b/src/box/txn.h
index e74c14d..a77189f 100644
--- a/src/box/txn.h
+++ b/src/box/txn.h
@@ -168,6 +168,12 @@ struct txn {
 	 /** Commit and rollback triggers */
 	struct rlist on_commit, on_rollback;
 	struct sql_txn *psql_txn;
+	/* Save all generated ids. */
+	struct {
+		int64_t size;
+		int64_t capacity;
+		int64_t *array;
+	} generated_ids;
 };
 
 /* Pointer to the current transaction (if any) */
diff --git a/test/sql/errinj.result b/test/sql/errinj.result
index a0ba60f..70aba9d 100644
--- a/test/sql/errinj.result
+++ b/test/sql/errinj.result
@@ -73,7 +73,8 @@ while f1:status() ~= 'dead' do fiber.sleep(0) end
 ...
 insert_res
 ---
-- rowcount: 1
+- generated_ids: []
+  rowcount: 1
 ...
 select_res
 ---
diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index af474bc..dd3fee1 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -69,19 +69,23 @@ type(ret.rows[1])
 -- Operation with rowcount result.
 cn:execute('insert into test values (10, 11, NULL)')
 ---
-- rowcount: 1
+- generated_ids: []
+  rowcount: 1
 ...
 cn:execute('delete from test where a = 5')
 ---
-- rowcount: 1
+- generated_ids: []
+  rowcount: 1
 ...
 cn:execute('insert into test values (11, 12, NULL), (12, 12, NULL), (13, 12, NULL)')
 ---
-- rowcount: 3
+- generated_ids: []
+  rowcount: 3
 ...
 cn:execute('delete from test where a = 12')
 ---
-- rowcount: 3
+- generated_ids: []
+  rowcount: 3
 ...
 -- SQL errors.
 cn:execute('insert into not_existing_table values ("kek")')
@@ -330,7 +334,8 @@ cn:execute('select :value', parameters)
 -- gh-2608 SQL iproto DDL
 cn:execute('create table test2(id primary key, a, b, c)')
 ---
-- rowcount: 1
+- generated_ids: []
+  rowcount: 1
 ...
 box.space.TEST2.name
 ---
@@ -338,7 +343,8 @@ box.space.TEST2.name
 ...
 cn:execute('insert into test2 values (1, 1, 1, 1)')
 ---
-- rowcount: 1
+- generated_ids: []
+  rowcount: 1
 ...
 cn:execute('select * from test2')
 ---
@@ -352,7 +358,8 @@ cn:execute('select * from test2')
 ...
 cn:execute('create index test2_a_b_index on test2(a, b)')
 ---
-- rowcount: 1
+- generated_ids: []
+  rowcount: 1
 ...
 #box.space.TEST2.index
 ---
@@ -360,7 +367,8 @@ cn:execute('create index test2_a_b_index on test2(a, b)')
 ...
 cn:execute('drop table test2')
 ---
-- rowcount: 1
+- generated_ids: []
+  rowcount: 1
 ...
 box.space.TEST2
 ---
@@ -370,100 +378,121 @@ box.space.TEST2
 -- Test CREATE [IF NOT EXISTS] TABLE.
 cn:execute('create table test3(id primary key, a, b)')
 ---
-- rowcount: 1
+- generated_ids: []
+  rowcount: 1
 ...
 -- Rowcount = 1, although two tuples were created:
 -- for _space and for _index.
 cn:execute('insert into test3 values (1, 1, 1), (2, 2, 2), (3, 3, 3)')
 ---
-- rowcount: 3
+- generated_ids: []
+  rowcount: 3
 ...
 cn:execute('create table if not exists test3(id primary key)')
 ---
-- rowcount: 0
+- generated_ids: []
+  rowcount: 0
 ...
 -- Test CREATE VIEW [IF NOT EXISTS] and
 --      DROP   VIEW [IF EXISTS].
 cn:execute('create view test3_view(id) as select id from test3')
 ---
-- rowcount: 1
+- generated_ids: []
+  rowcount: 1
 ...
 cn:execute('create view if not exists test3_view(id) as select id from test3')
 ---
-- rowcount: 0
+- generated_ids: []
+  rowcount: 0
 ...
 cn:execute('drop view test3_view')
 ---
-- rowcount: 1
+- generated_ids: []
+  rowcount: 1
 ...
 cn:execute('drop view if exists test3_view')
 ---
-- rowcount: 0
+- generated_ids: []
+  rowcount: 0
 ...
 -- Test CREATE INDEX [IF NOT EXISTS] and
 --      DROP   INDEX [IF EXISTS].
 cn:execute('create index test3_sec on test3(a, b)')
 ---
-- rowcount: 1
+- generated_ids: []
+  rowcount: 1
 ...
 cn:execute('create index if not exists test3_sec on test3(a, b)')
 ---
-- rowcount: 0
+- generated_ids: []
+  rowcount: 0
 ...
 cn:execute('drop index test3_sec on test3')
 ---
-- rowcount: 1
+- generated_ids: []
+  rowcount: 1
 ...
 cn:execute('drop index if exists test3_sec on test3')
 ---
-- rowcount: 0
+- generated_ids: []
+  rowcount: 0
 ...
 -- Test CREATE TRIGGER [IF NOT EXISTS] and
 --      DROP   TRIGGER [IF EXISTS].
 cn:execute('create trigger trig INSERT ON test3 BEGIN SELECT * FROM test3; END;')
 ---
-- rowcount: 1
+- generated_ids: []
+  rowcount: 1
 ...
 cn:execute('create trigger if not exists trig INSERT ON test3 BEGIN SELECT * FROM test3; END;')
 ---
-- rowcount: 0
+- generated_ids: []
+  rowcount: 0
 ...
 cn:execute('drop trigger trig')
 ---
-- rowcount: 1
+- generated_ids: []
+  rowcount: 1
 ...
 cn:execute('drop trigger if exists trig')
 ---
-- rowcount: 0
+- generated_ids: []
+  rowcount: 0
 ...
 -- Test DROP TABLE [IF EXISTS].
 -- Create more indexes, triggers and _truncate tuple.
 cn:execute('create index idx1 on test3(a)')
 ---
-- rowcount: 1
+- generated_ids: []
+  rowcount: 1
 ...
 cn:execute('create index idx2 on test3(b)')
 ---
-- rowcount: 1
+- generated_ids: []
+  rowcount: 1
 ...
 box.space.TEST3:truncate()
 ---
 ...
 cn:execute('create trigger trig INSERT ON test3 BEGIN SELECT * FROM test3; END;')
 ---
-- rowcount: 1
+- generated_ids: []
+  rowcount: 1
 ...
 cn:execute('insert into test3 values (1, 1, 1), (2, 2, 2), (3, 3, 3)')
 ---
-- rowcount: 3
+- generated_ids: []
+  rowcount: 3
 ...
 cn:execute('drop table test3')
 ---
-- rowcount: 1
+- generated_ids: []
+  rowcount: 1
 ...
 cn:execute('drop table if exists test3')
 ---
-- rowcount: 0
+- generated_ids: []
+  rowcount: 0
 ...
 --
 -- gh-2948: sql: remove unnecessary templates for binding
@@ -535,7 +564,8 @@ cn = remote.connect(box.cfg.listen)
 ...
 cn:execute('create table test (id integer primary key, a integer, b integer)')
 ---
-- rowcount: 1
+- generated_ids: []
+  rowcount: 1
 ...
 future1 = cn:execute('insert into test values (1, 1, 1)', nil, nil, {is_async = true})
 ---
@@ -548,7 +578,8 @@ future3 = cn:execute('insert into test values (2, 2, 2), (3, 3, 3)', nil, nil, {
 ...
 future1:wait_result()
 ---
-- rowcount: 1
+- generated_ids: []
+  rowcount: 1
 ...
 future2:wait_result()
 ---
@@ -558,7 +589,8 @@ future2:wait_result()
 ...
 future3:wait_result()
 ---
-- rowcount: 2
+- generated_ids: []
+  rowcount: 2
 ...
 future4 = cn:execute('select * from test', nil, nil, {is_async = true})
 ---
@@ -580,6 +612,81 @@ 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)')
+---
+- generated_ids: []
+  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')
+---
+- generated_ids: []
+  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)')
+---
+- generated_ids: []
+  rowcount: 1
+...
+cn:execute('drop table test2')
+---
+- generated_ids: []
+  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
 
-- 
2.7.4